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

Allow directory with template overrides #9

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion tifeatures/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def bbox_query(
raise Exception("Invalid BBOX.")

bounds = tuple(map(float, split_bbox))
return Polygon.from_bounds(*bounds)
return Polygon.from_bounds(*bounds) # type: ignore

return None

Expand Down
70 changes: 58 additions & 12 deletions tifeatures/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import json
import pathlib
from dataclasses import dataclass, field
from typing import Any, Callable, Optional
from os import PathLike
from typing import Any, Callable, List, Optional, Union

import jinja2
from geojson_pydantic.geometries import Polygon

from tifeatures import model
Expand All @@ -16,20 +18,65 @@
from tifeatures.settings import APISettings

from fastapi import APIRouter, Depends, Path, Query
from fastapi.templating import Jinja2Templates

from starlette.datastructures import QueryParams
from starlette.requests import Request
from starlette.responses import HTMLResponse
from starlette.templating import Jinja2Templates, _TemplateResponse

if hasattr(jinja2, "pass_context"):
pass_context = jinja2.pass_context # type: ignore
else: # pragma: nocover
pass_context = jinja2.contextfunction # type: ignore[attr-defined]

template_dir = str(pathlib.Path(__file__).parent.joinpath("templates"))
templates = Jinja2Templates(directory=template_dir)
settings = APISettings()


class SearchPathTemplates(Jinja2Templates):
Copy link
Member

Choose a reason for hiding this comment

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

what about something a bit simpler:

template_dir = str(pathlib.Path(__file__).parent.joinpath("templates"))
templates = Jinja2Templates(directory=template_dir)

def create_template_response(name: str, context: dict, **kwargs: Any):
    if settings.template_directory:
        custom_templates = Jinja2Templates(directory=settings.template_directory)
        try:
            return custom_templates.TemplateResponse(name, context, **kwargs)
        except:  # TODO Add more precise exception handling
            pass

    return templates.TemplateResponse(name, context, **kwargs)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thing i like about extending SearchPathTemplates is that it allows us to use the native tooling in jinja and could handle several paths if anyone needed as well.

Copy link
Member

Choose a reason for hiding this comment

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

👍 it seems you can do this without re-writing your own class encode/starlette#1214

"""
templates = DefaultTemplates("templates")
return templates.TemplateResponse("index.html", {"request": request})
"""

def __init__(
self,
directory: Union[str, PathLike, List[Union[str, None, PathLike]]],
**env_options: Any,
) -> None:
"""Initialize Search Path Template."""
assert jinja2 is not None, "jinja2 must be installed to use Jinja2Templates"
self.env = self._create_env(directory, **env_options)

def _create_env(
self,
directory: Union[str, PathLike, List[Union[str, None, PathLike]]],
**env_options: Any,
) -> "jinja2.Environment":
@pass_context
def url_for(context: dict, name: str, **path_params: Any) -> str:
request = context["request"]
return request.url_for(name, **path_params)

if isinstance(directory, list):
loader = jinja2.FileSystemLoader([d for d in directory if d])
else:
loader = jinja2.FileSystemLoader(directory)
env_options.setdefault("loader", loader)
env_options.setdefault("autoescape", True)

env = jinja2.Environment(**env_options)
env.globals["url_for"] = url_for
return env


default_template_dir = str(pathlib.Path(__file__).parent.joinpath("templates"))
templates = SearchPathTemplates(
directory=[settings.template_directory, default_template_dir]
)


def create_html_response(
request: Request, data: str, template_name: str
) -> HTMLResponse:
) -> _TemplateResponse:
Copy link
Member

Choose a reason for hiding this comment

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

👍

"""Create Template response."""
urlpath = request.url.path
crumbs = []
Expand Down Expand Up @@ -480,9 +527,8 @@ async def items(

if (matched_items - items_returned) > offset:
next_offset = offset + items_returned
query_params = QueryParams(
{**request.query_params, "offset": next_offset}
)
query_params = {**request.query_params, "offset": next_offset}

url = (
self.url_for(request, "items", collectionId=collection.id)
+ f"?{query_params}"
Expand All @@ -496,13 +542,13 @@ async def items(
query_params.pop("offset")
prev_offset = max(offset - items_returned, 0)
if prev_offset:
query_params = QueryParams({**query_params, "offset": prev_offset})
query_params = {**query_params, "offset": prev_offset}
else:
query_params = QueryParams({**query_params})
query_params = {**query_params}

url = self.url_for(request, "items", collectionId=collection.id)
if query_params:
url += f"?{query_params}"
url += f"?{QueryParams(**query_params)}"
Copy link
Member

Choose a reason for hiding this comment

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

is this related to the PR topic?

I'm fine with the change, but I worry that I did use this kind of style at multiple places 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, mypy didn't like that query_params was a dict in some places and QueryParama elsewhere


links.append(
model.Link(href=url, rel="prev", type=MediaType.geojson),
Expand Down
6 changes: 3 additions & 3 deletions tifeatures/layer.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from buildpg import asyncpg, clauses
from buildpg import funcs as pg_funcs
from buildpg import render
from geojson_pydantic import Feature, FeatureCollection
from geojson_pydantic.features import Feature, FeatureCollection
from pydantic import BaseModel, Field, root_validator


Expand Down Expand Up @@ -41,7 +41,7 @@ async def features(
limit: int = None,
offset: int = None,
**kwargs: Any,
) -> Items:
) -> FeatureCollection:
Copy link
Member

Choose a reason for hiding this comment

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

?
Items is a FeatureCollection based model

class Items(FeatureCollection):
"""Items model
Ref: http://schemas.opengis.net/ogcapi/features/part1/1.0/openapi/schemas/featureCollectionGeoJSON.yaml
"""
id: str
title: Optional[str]
description: Optional[str]
keywords: Optional[List[str]]
features: List[Item] # type: ignore
links: Optional[List[Link]]
timeStamp: Optional[str]
numberMatched: Optional[int]
numberReturned: Optional[int]
class Config:
"""Link model configuration."""
arbitrary_types_allowed = True

(Gives better documentation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mypy was complaining, i was just trying to get the tests to pass (same with everything in that last commit).

Copy link
Member

Choose a reason for hiding this comment

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

👍 I'll check that 🙏

"""Return a FeatureCollection."""
...

Expand Down Expand Up @@ -85,7 +85,7 @@ async def features(
limit: int = None,
offset: int = None,
**kwargs: Any,
) -> Items:
) -> FeatureCollection:
"""Return a FeatureCollection."""

limit = limit or 10
Expand Down
2 changes: 1 addition & 1 deletion tifeatures/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from typing import List, Optional

from geojson_pydantic import Feature, FeatureCollection
from geojson_pydantic.features import Feature, FeatureCollection
from pydantic import BaseModel

from tifeatures.resources.enums import MediaType
Expand Down
1 change: 1 addition & 0 deletions tifeatures/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class _APISettings(pydantic.BaseSettings):
name: str = "TiFeatures"
cors_origins: str = "*"
cachecontrol: str = "public, max-age=3600"
template_directory: Optional[str] = None

@pydantic.validator("cors_origins")
def parse_cors_origin(cls, v):
Expand Down