Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make http error cache control behavior configurable. #444

Merged
merged 2 commits into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/titiler/core/tests/test_cache_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from fastapi import FastAPI, Path

from starlette.responses import Response
from starlette.testclient import TestClient


Expand Down Expand Up @@ -36,9 +37,19 @@ async def tiles(
"""tiles."""
return "yeah"

@app.get("/emptytiles/{z}/{x}/{y}")
async def emptytiles(
z: int = Path(..., ge=0, le=30, description="Mercator tiles's zoom level"),
x: int = Path(..., description="Mercator tiles's column"),
y: int = Path(..., description="Mercator tiles's row"),
):
"""tiles."""
return Response(status_code=404)

app.add_middleware(
CacheControlMiddleware,
cachecontrol="public",
cachecontrol_http_code_range=400,
exclude_path={r"/route1", r"/route2", r"/tiles/[0-1]/.+"},
)

Expand All @@ -58,3 +69,6 @@ async def tiles(

response = client.get("/tiles/3/1/1")
assert response.headers["Cache-Control"] == "public"

response = client.get("/emptytiles/3/1/1")
assert not response.headers.get("Cache-Control")
7 changes: 6 additions & 1 deletion src/titiler/core/titiler/core/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def __init__(
self,
app: ASGIApp,
cachecontrol: Optional[str] = None,
cachecontrol_http_code_range: Optional[int] = 500,
exclude_path: Optional[Set[str]] = None,
) -> None:
"""Init Middleware.
Expand All @@ -31,6 +32,7 @@ def __init__(
"""
super().__init__(app)
self.cachecontrol = cachecontrol
self.cachecontrol_http_code_range = cachecontrol_http_code_range
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the name of the parameter. To me if we say Range I'm assuming the input to be a sequence (min, max), while here in reality we want the upper value of status_code.

what do you think about cachecontrol_max_http_code ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strong agree 👍 .

self.exclude_path = exclude_path or set()

async def dispatch(self, request: Request, call_next):
Expand All @@ -41,7 +43,10 @@ async def dispatch(self, request: Request, call_next):
if re.match(path, request.url.path):
return response

if request.method in ["HEAD", "GET"] and response.status_code < 500:
if (
request.method in ["HEAD", "GET"]
and response.status_code < self.cachecontrol_http_code_range
):
response.headers["Cache-Control"] = self.cachecontrol

return response
Expand Down