#360 Routing based on values in designated upstream request headers

[mrclayman]

Closes #360

• Routing based on Request Header #360

Addition of support for request re-routing based also on upstream header configuration as per issues

• Routing based on Request Header #360
• ReRoute by header value #624

Proposed Changes

• Implementation of support for re-routing based on request's header configuration
• Update of relevant tests to cover the new functionality
• Update of relevant documentation to make clear how to make use of the new functionality

Downstream URL placeholder replacement mentioned in issue #360 is not yet supported although I do have plans to add support for that as well.

[raman-m]

Zbyněk, thanks for the great PR! 💪

Does the PR close the #624 ?
I know it is closed, but maybe some implementations of #624 are included into this PR ?!

---

I am a bit confused by this PR because you said that it makes little sense to me to keep working on this any longer.
Anyway, I welcome your PR.
You need to understand that code review of this PR will take more time as for regular one because both solutions should be compared after a classic review.
This situation of having 2 solutions for the same issue is quite strange. 😄 But it is OK, we will have this experience. 😉

[raman-m]

Zbyněk,
Please request code review manually from the people you've communicated to in #360, if it is required on your opinion.
As for me it is hard to track your network.

[mrclayman]

Well, you wanted to salvage my changes from the old branch, so I thought it shouldn't hurt to just to take them and put them in a new branch. 😄 At any rate, I have been thinking about the practical benefit of the current solution. You see, since users will still have to provide unique upstream URL for different downstreams, I thought that maybe we could consider allowing identical upstream URL, but differentiating the mappings based on the headers.It kind of makes sense in my head, but let me know what you and maybe also other maintainers think.

EDIT: Actually, just looking at the first post in #360 , it becomes clear that the above is a requirement, not just an option. I will have to look into allowing it as it looks like Ocelot currently won't allow multiple upstream routes to be the same.

[mrclayman]

Cheers for the feedback, @RaynaldM . I have integrated it along with a modification to the FileConfigurationFluentValidator to allow for multiple routes having the same upstream path but different routing header layout. Seems to work nicely on my end but please do check out the changes and let me know if anything doesn't sit. In the meantime, I should probably look into placeholder replacement as some people voiced interest in having that working with headers also.

[mrclayman]

Having taken a look at how URL placeholder extraction and replacement works, it's quite obvious that the current algorithm (implemented in UrlPathPlaceholderNameAndValueFinder) has been designed to work with URL's (routes and query arguments), which is suboptimal for header values as those usually don't contain either. I suppose I would therefore look into adapting some of the parts of the parser for use outside of URL patterns so that they could be reused in header value parser as well. It's just an initial idea that I have not thought a lot about, but let me know your take so we can agree on something. 👍

[raman-m]

@mrclayman Hi Zbyněk!
Some suggestions:

• Use file-scoped namespaces for new added files always, because we have .NET 6+ environment
• Remove and Sort Usings by standard refactoring action in Visual Studio. I see you use some other RAD tool.
• Use var option to define a variable for reference types

---

Don't judge me much while resolving these issues please! 🤣 👇

---

Helpful unit tests:

• Issue 1 → Ocelot-1684-r1293789996 Unit Tests | C# Online Compiler | .NET Fiddle

[raman-m]

@mrclayman commented on Aug 7, 2023

This is crazy idea!
I saw a lot of PRs and issues in the backlog which change and/or fix the logic of placeholders. So, placeholders-related logic is quite unstable, it has some unclear bug fixes, and not documented well...
Your refactoring idea can be a challenge for you! 🤣
Should I say "Good luck"? 😄

---

Why not to fix code review issues first? 😉
And, don't forget about @RaynaldM's code review suggestions too! 😺

[mrclayman]

@raman-m requested changes on Aug 14

I work in Linux. There is no Visual Studio for that. I have already had to disable .editorconfig because apparently Rider cares about it a lot more than Visual Studio judging by the awful mixture of line endings across the code base. I'll see if I can set Rider to not reorder usings but can't make any promises about that.

[mrclayman]

Integrated your PR feedback, @raman-m . See 6d29040 and let me know what you think. I tried to cover all of it so here's hoping I have succeeded at that. 🤞

[mrclayman]

Ping @raman-m, hope you didn't fall off the edge of the earth. 🙂