search

ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.24k stars 1.62k forks source link

`FileCacheOptions` not working after the header was introduced in FileCache settings in version 23.0.0 #2059

Closed SheruGaur closed 2 months ago

SheruGaur commented 2 months ago

Discussed in https://github.com/ThreeMammals/Ocelot/discussions/2054

Hi

We are facing issue regarding cache option, however in earlier version till v22.0.1 FileCacheOptions was working fine but after the release of v23.0.0 we are facing issue mentioned below in two cases.

Case 1: V.22.0.1 "FileCacheOptions": { "TtlSeconds": 15, "Region": "europe-central" }

It was working fine with the multiple requests. before the latest update case2: After the addition of new parameter named Header in FileCacheOptions there, we are facing issue on multiple requests in case one request is already generated and in the same configured cache expiry time if another request come with different values, then we are getting same response as per the first request.

Case 2: v23.2.2 "FileCacheOptions": { "TtlSeconds": 15, "Region": "europe-central" }

As per documentation header is not mandatory, hence the cache option should work without header also, which is not the case.

Observed Behaviour

  1. File cache returns same data for different request payload on same Route in new version 23.2.2 (we tested in this version directly)
  2. File cache returns different data for different request payload on same Route in version 22.0.1.

Please can you help us fix this as this seems to be a breaking change since version 23.0.0 as mentioned by Raman in the discussion.

Thanks, Sheru Kumar Gaur

sberwal commented 2 months ago

Hi Team

we have also observed the same behaviour. It seems the request payload is not considered when caching the data.

Hence cache returns the data for 1st request payload from the cache, for subsequent requests with different request payload.

This is happening when Header is not set!

Thanks Suraj

raman-m commented 2 months ago

@SheruGaur

Please can you help us fix this as this seems to be a breaking change since version 23.0.0 as mentioned by Raman in the discussion.

Certainly...

  1. Can you provide the specific lines of code where the breaking change occurs?
  2. We are willing to assist you if you can contribute further by demonstrating the precise breaking change in a PR, could you do that?
ggnaegi commented 2 months ago

@raman-m it's probably related to the fact that the body hashing is now disabled by default and it must be activated explicitly. But, as mentioned by @thiagoloureiro, the needed property isn't available in file configuration. So, I would recommend to deal with this issue before anything else...

raman-m commented 2 months ago

@ggnaegi commented on May 9

Understood! I guess we need to link the PR 2058 ... The bug fix will be released in current milestone. I don't want to make separate hotfix release: developers may to rollback to previous versions temporarily. So, the bugfix will be a part of version 23.3

P.S.

https://github.com/ThreeMammals/Ocelot/blob/aef3e6b9f53ad8c8aa8c9ade92a572f60d0672a1/src/Ocelot/Configuration/File/FileRoute.cs#L45 Firstly added in commit https://github.com/ThreeMammals/Ocelot/commit/47afc850ff1aa48f4ec0d88ec5a309c02992c7f6#diff-e1943063e63a6438191010c566111bea5c47f4cb063c04c7cc6c91a8c4267812 by Tom many many years ago. I wonder the property was named with File prefix. And the most appropriate name is CacheOptions. Will renaming be a breaking change? It will be better to rename :point_right: 

- public FileCacheOptions FileCacheOptions { get; set; }
+ public FileCacheOptions CacheOptions { get; set; }