more cache-control cleanup

[davidism]

This has been on my mind for years. Every time something about the cache-control header comes up and I look at the code, it struck me that there were weird types and values being assigned to attributes. closes #2980

Originally, I wanted to ditch cache_control_property and implement everything as direct properties, or a few descriptors for different types instead of the overloaded get/set helpers that handle a bunch of different scenarios. I've decided to hold off on this for now. cache_control_property is useful since custom attributes are part of the spec. The arguments do represent the different cases: a key present without a value, a value of a specific type. I've added a better docstring generator for the properties, so the types and behavior of the attribute gets documented instead of "accessor for max-age". It's also possible to pass in a custom doc for cases where the generator isn't enough, such as for max_stale.

This consolidates and organizes the changelog entries for #2881 and #2948. #2881 already began fixing some types/values, and this is going in a feature release, so I felt comfortable enough extending this to the rest of the properties. There was no obvious way to deprecate the old behaviors, but searches suggest they weren't being used often, or in ways that would be affected. The fix if something does break is relatively simple, changing what value is being tested.

Dict values are always str | None. Previously, setting typed properties would set the typed value in the dict, which only worked because converting to a header converted to string. Now they're converted to their type, then to a string. This makes the type annotation when working with the dict interface more useful.

Setting a property to False is equivalent to setting it to None. This makes ResponseCacheControl.no_cache = True type check properly even thought it ends up being a string value.

Getting typed properties will return None if conversion raises ValueError, rather than the string. This is consistent with how MultiDict.get(type=) works.

max_age is None if not present, rather than -1. This makes it consistent with other properties, and makes more sense in a boolean context.

no_cache was previously a string for both request and response. But the spec says that it is a boolean with no value for requests. For requests, it is False rather than "*" when not present.

max_stale is a weird part of the spec. It can either be present with no value, have an int value, or not be present. The previous implemenation treated it as int | str | None, where the only string value was "*" if it was present with no value. As explained in #2980, this does not make type checkers happy. Split this into two properties, max_stale is the int value if present, or None if not present or no value. max_stale_any is a bool that only indicates if it was present or not, regardless of value.

#2881 made no_transform is a boolean. Previously it was mistakenly always None.

#2881 made min_fresh None if not present instead of "*".

private is a boolean, it is False instead of "*" when not present.

#2881 added the must_understand property.

#2948 added the stale_while_revalidate and stale_if_error properties. This PR moves stale_if_error to be on both request and response.

#2881 and this PR generally cleaned up type annotations.

[RazerM]

It returned * when present:

3.0.6:

>>> assert parse_cache_control_header("", cls=RequestCacheControl).no_cache is None
>>> assert parse_cache_control_header("no-cache", cls=RequestCacheControl).no_cache == '*'

main:

>>> assert parse_cache_control_header("", cls=RequestCacheControl).no_cache is False
>>> assert parse_cache_control_header("no-cache", cls=RequestCacheControl).no_cache is True

[RazerM]

It returned -1 when present.

3.0.6:

>>> assert parse_cache_control_header("", cls=RequestCacheControl).max_age is None
>>> assert parse_cache_control_header("max-age", cls=RequestCacheControl).max_age == -1
>>> assert parse_cache_control_header("max-age=0", cls=RequestCacheControl).max_age == 0

main:

>>> assert parse_cache_control_header("", cls=RequestCacheControl).max_age is None
>>> assert parse_cache_control_header("max-age", cls=RequestCacheControl).max_age is None
>>> assert parse_cache_control_header("max-age=0", cls=RequestCacheControl).max_age == 0

[davidism]

There is something wrong, but it's that the previous was "when present without a value, which is not valid."

[RazerM]

It returned * when present.

3.0.6:

>>> assert parse_cache_control_header("", cls=RequestCacheControl).min_fresh is None
>>> assert parse_cache_control_header("min-fresh", cls=RequestCacheControl).min_fresh == '*'
>>> assert parse_cache_control_header("min-fresh=1", cls=RequestCacheControl).min_fresh == 1

main:

>>> assert parse_cache_control_header("", cls=RequestCacheControl).min_fresh is None
>>> assert parse_cache_control_header("min-fresh", cls=RequestCacheControl).min_fresh is None
>>> assert parse_cache_control_header("min-fresh=1", cls=RequestCacheControl).min_fresh == 1

[davidism]

There is something wrong, but it's that the previous was "when present without a value, which is not valid."

[RazerM]

It returned * when present without an argument. It should not be bool since arguments are permitted:

3.0.6:

>>> assert parse_cache_control_header("", cls=ResponseCacheControl).private is None
>>> assert parse_cache_control_header("private", cls=ResponseCacheControl).private == '*'
>>> assert parse_cache_control_header('private="Set-Cookie, Authorization"', cls=ResponseCacheControl).private == 'Set-Cookie, Authorization'

main:

>>> assert parse_cache_control_header("", cls=ResponseCacheControl).private is False
>>> assert parse_cache_control_header("private", cls=ResponseCacheControl).private is True
>>> cc = parse_cache_control_header('private="Set-Cookie, Authorization"', cls=ResponseCacheControl)
>>> assert cc.private == 'Set-Cookie, Authorization', f"{cc.private=}"
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AssertionError: cc.private=True

[davidism]

Totally right, I missed that it can have an argument. It's because I was toggling between the spec and MDN, which doesn't mention the argument https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control.

[davidism]

I also realized that instead of splitting into two properties max_stale and max_stale_any, I can type max_stale: int | Literal["*"] | None then type checkers should understand that != "*" narrows the type. Or could use Literal[True].