#1172 Default CacheKeyGenerator

ThreeMammals / Ocelot

.NET API Gateway

https://www.nuget.org/packages/Ocelot

MIT License

8.31k stars 1.63k forks source link

#1172 Default CacheKeyGenerator #1849

Closed raman-m closed 9 months ago

raman-m commented 9 months ago

Follows up #1172

1172

Related to #1808

1808

During 22.0 release feature #1172 was excluded from delivery list. Now it is time to deliver it once again with some enhancements after team's discussion (start).

Proposed Changes

Disable request content hashing for performance reasons
Introduce new EnableRequestBodyHashing property of the CacheOptions class
Make CacheKeyGenerator default via class renaming

raman-m commented 9 months ago

@ggnaegi You are welcome to review code of this PR!

We have some dev plan on this:

your comments: 1st and 2nd
after merging this PR you'll create new PR on top of this code

Please review! FYI I didn't extract the feature from DI aka Ocelot Builder. But it should be. Hope you will do that in the next PR. The scope of work of this PR is delivery of Mohsen's feature with disabling ugly content hashing: no more.

raman-m commented 9 months ago

Should I add acceptance tests? : 😇 :yum:

ggnaegi commented 9 months ago

Should I add acceptance tests? : 😇 😋

😇

raman-m commented 9 months ago

@ggnaegi suggested changes on Dec 9 @RaynaldM approved these changes on Dec 11

Code review issues have been fixed ✅ Caching Header is a new feature, so seems I have to add acceptance tests... But I've done with unit tests, they are written.

And could you review docs once again please?

raman-m commented 9 months ago

@ggnaegi commented on Dec 9

Acceptance test has been added! ✔️ See commit https://github.com/ThreeMammals/Ocelot/pull/1849/commits/3d536bee92942f8c2058bbf84522fda188b40c6e By default body content is not hashed.

@ggnaegi Looks good now?

raman-m commented 9 months ago

Ready for delivery! ✔️