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

The caching algorithm should observe the max-age headers #433

Merged
merged 1 commit into from
Jan 16, 2022

Conversation

ashleyfrieze
Copy link
Contributor

This is the foremost priority, dropping back to Last-Modified with a heuristic as per RFC7234

This replaces the behaviour where the max-age headers were not considered at the time of caching, and where very recently Last-Modified pages were NOT allowed in the cache either way.

This change introduces some additional tests, with some test refactoring to avoid too many repeated constants.

It also centralises the logic for eviction and ingress into the cache.

It's a potential fix for #432

@rbri
Copy link
Member

rbri commented Jan 15, 2022

Great thanks, can you please add your real name to the list of authors for both changed files.

@rbri
Copy link
Member

rbri commented Jan 15, 2022

and on my machine contentWithLastModifiedTimeIsCachedAfterAFewPercentOfCreationAge fails - maybe because of date format issues (only a wild guess have not analyzed this)

…remost, dropping back to `Last-Modified` with a heuristic as per RFC7234
@ashleyfrieze
Copy link
Contributor Author

@rbri test failure was a minor test design issue - mixing hard coded time with calculated times. My mistake. I was rushing on a Friday evening. I've changed it to use per test constants relating to now and that should sort it out.

I've also modified the authors as requested.

@rbri rbri merged commit ad602d4 into HtmlUnit:master Jan 16, 2022
@rbri
Copy link
Member

rbri commented Jan 16, 2022

many thanks, feel free to provide more of this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants