#2002 Early removal of a replaced placeholder parameter in `DownstreamUrlCreatorMiddleware`

[bbenameur]

Fixes #2002

• 2002

Proposed Changes

Remove only added query to placeholders.

[raman-m]

I answered you in #2002, see link! This PR fixes your rare user case when you wanted just use the same names of placeholder and query param. So, you wanted to use userId placeholder in query string like ?userId={userId}. Ocelot has a list of other features in Placeholders logic. And probably your PR will break them... Placeholders logic is quite complex and stable now, and we don't want to change the logic at all! Read the docs for correct usage.

As a hotfix DevOps you need to rollback to version 21.0 as I suggested to you.

If you need something more, then just fork Ocelot repo and develop whatever you want! Good luck!

[raman-m]

Strange... the build 3669 is 🟢 That means the unit tests were passed. So, that means there is no much sense in moving of the code block...

[raman-m]

Once again... In #2002, I previously explained how to utilize the new version 22.0+. It appears that your route templates are currently invalid. Instead, consider leveraging the new behavior of the Placeholders feature. While I don’t observe any regressions when used correctly, it’s possible that we may have introduced a breaking change without adequately notifying the community. As a result, I’ll make a note of this incident to ensure that we always include Breaking Changes in the Release Notes. 📝

[bbenameur]

Strange... the build 3669 is 🟢 That means the unit tests were passed. So, that means there is no much sense in moving of the code block...

The build 3669 is green because the new code have no regression and work correctly, but we need some new test that cover the case where parameter name and value are some like userId=userId

[raman-m]

but we need some new test that cover the case where param name and value are some like userId=userId

🆗 Work more! Our dev process requires tests for sure. Include unit tests and one acceptance test please. But you must understand that requirement userId={userId} is wrong because param will be removed from query string according to current Placeholders logic! I've already explained you in 2002 that you have to change your routes like: userId={user} which allows the param be kept in the query. If you will adapt your route templates then you will get exactly what you need.

[raman-m]

@ggnaegi left a comment

Do you approve or not?