DRAFT Fix pagination links wrt override of url_for

[philvarner]

What I'm changing

How I did it

Checklist

• Tests pass: uv run pytest
• Checks pass: uv run pre-commit run --all-files
• CHANGELOG is updated (if necessary)

[jkeifer]

Maybe self.url_for shouldn't be returning a string? If so then I think maybe you do this:

href = str(
    self.url_for(
        request,
        f"{self.name}:{ROOT}"
     ).include_query_params(
        next=pagination_token,
        limit=limit,
     ),
)

The need to stringify the the URL is not because the Link class actually wants a string. The problem is that the model uses the pydantic type AnyUrl where FastAPI is built on starlette, the latter of which has its own URL class that is not directly compatible with AnyUrl. For that matter, str isn't exactly either, but note the Link model is defined like so:

class Link(BaseModel):
    href: AnyUrl
    rel: str
    type: str | None = None
    title: str | None = None
    method: str | None = None
    headers: dict[str, str | list[str]] | None = None
    body: Any = None

    model_config = ConfigDict(extra="allow")

    # redefining init is a hack to get str type to validate for `href`,
    # as str is ultimately coerced into an AnyUrl automatically anyway
    def __init__(self, href: AnyUrl | str, **kwargs: Any) -> None:
        super().__init__(href=href, **kwargs)

That __init__ allows us to use str as the serialized URL format to convert between starlette's URL type and AnyUrl, because AnyUrl will convert str instances to itself (assuming the str is a valid URL). What we could do here to obviate the need for every call site to convert starlette URLs to str might be something like this:

    def __init__(self, href: Any, **kwargs: Any) -> None:
        if not isinstance(href, (AnyUrl, str)):
            href = str(href)
        super().__init__(href=href, **kwargs)

With this new transformation happening in the __init__, it feels like a before validator on the href field would be a better approach. Except I don't think before validators will change the __init__ signature so type checking would not allow anything but AnyUrl without an override. But I also think the kwargs as written is not ideal because that is also breaking type checking for the other fields??? Whatever I guess, that's an existing issue no one is complaining about, we can ignore it for now unless we want to better type the __init__ override.

To summarize: I don't love doing replace when instances not overriding url_for will run the replace without any change to the generated href. So I'm looking to a solution that can avoid that extra operation without having to re-insert the str() call everywhere it has been removed.

[philvarner]

yeah, i thought about that -- i think i was trying to make the minimal change that could work, and now force users to change how they're overriding url_for -- but I think i'm the only consumer of this so far, so I should just do it this way -- thanks for the encouragement.