1.6.40 skips cache revalidation when it may be required about souin HOT 9 CLOSED

You're right, my bad, I'll fix that too for the next release (I hope to release this week). Thank you for this feedback.

from souin.

Hello @let4be, thank you for your feedback. Can you send an example to reproduce please ?

from souin.

Sorry, I don't have an example.

I was just going through the code and noticed there's no longer a .Revalidate() call in this particular if branch: https://github.com/darkweak/souin/blob/v1.6.40/pkg/middleware/middleware.go#L399

Meaning we will NEVER revalidate with the upstream if response was located in the cache, which is a drastic change introduced in this PR

from souin.

This if condition is for the fresh cache, it doesn't need to revalidate here unlike the stale cache condition https://github.com/darkweak/souin/blob/v1.6.40/pkg/middleware/middleware.go#L428 which have the Revalidate call https://github.com/darkweak/souin/blob/v1.6.40/pkg/middleware/middleware.go#L451. Maybe I'm missed something in the RFC.

from souin.

Revalidation is related to the conditional request, as far as I understood we cannot always know that cached version is fresh if client is using one of conditional requests(https://datatracker.ietf.org/doc/html/rfc7232)

Before this PR there was a .Revalidate() call in this if branch...

from souin.

@let4be Can you try with that commit a36be95b162f7847590d01a0d5c37a1abd4fd833 and tell me if it's all good now?

from souin.

Commented here: a36be95

from souin.

Fixed in a9569b828220655ca1039216cd02a75a1d67eed2

from souin.

Looks better!
I'm still not exactly sure if we update cached headers when remote said NotModified(and headers only) and if we reuse cached body in this case with new headers

from souin.