🐛 Cache Middleware - Update expiration for expired entry

[defshift]

Please provide enough information so that others can review your pull request:
I noticed that expiration timestamp is still the same after cache entry has been expired. It leads to situation when right after the entry has been expired, all the next requests that were made right after the eviction are not cached since the expiration time is always behind the expiration deadline.

-> call - miss
-> call - hit
-> call - hit
-> call - hit
...
(wait for expiration)
...
(now we'll make the calls right after each other)
-> call - miss
-> call - miss
-> call - miss
....

This PR contains one-liner fix and updated test.

[welcome]

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

[ReneWerner87]

Thanks for the pull request, after seeing the point now, I wonder why we don't put the expiration time set at the bottom, after the check of the storages, when the other properties of the e record are set.

[ReneWerner87]

I mean shortly before we set e.body

[defshift]

@ReneWerner87 this is a good point. Updated the PR

[ReneWerner87]

Thx

[ReneWerner87]

Can you check the tests again, by the way, I also meant line 66, that could be omitted if you formulate a suitable condition for the else and the elseif.

[defshift]

@ReneWerner87 right, updated the PR

[ReneWerner87]

I think the first condition needs also the
0 != e.exp

[ReneWerner87]

Perfect thx

[defshift]

@ReneWerner87 oh, right. And this is a reminder that need to prepare PRs with a fresh mind :D

[ReneWerner87]

Maybe we should use

else if ts < e.exp

What do you think

[ReneWerner87]

Go to sleep now, look at it tomorrow and merge it, if the tests are okay

[defshift]

Sounds like a plan ;)

[welcome]

Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

[defshift]

@ReneWerner87 thanks. Noticed that the test has failed for one environment. I bet it's because memory storage managed to delete old entry before the next check https://github.com/gofiber/fiber/blob/master/internal/memory/memory.go#L72

[defshift]

I can add an optional parameter for gcInterval to https://github.com/gofiber/fiber/blob/master/internal/memory/memory.go#L20
What do you think @ReneWerner87?

[ReneWerner87]

yeah, sound good

the failure was because one had made exactly in the time of the intervals actions of the test ?

[defshift]

yeah, exactly. And this looks like a race condition. But now I'm thinking if changing of memory storage gc interval is a really good solution

[defshift]

@ReneWerner87 ok, logs showed that it's not related to the memory storage. Will look into it this evening