-
Notifications
You must be signed in to change notification settings - Fork 2.1k
App Config - Query Param Ordering #46665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new pipeline policy to normalize query parameters for Azure App Configuration requests to support Azure Front Door as a CDN. The policy ensures query parameter keys are converted to lowercase and sorted alphabetically.
Key Changes:
- Added a new
QueryParamPolicythat normalizes query parameters by converting keys to lowercase and sorting them alphabetically - Integrated the policy into the HTTP pipeline in
ConfigurationClientBuilder - Added comprehensive test coverage for various query parameter scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| QueryParamPolicy.java | New pipeline policy implementation that normalizes query parameters |
| QueryParamPolicyTest.java | Comprehensive test suite covering various query parameter scenarios |
| ConfigurationClientBuilder.java | Integration of the new policy into the HTTP pipeline |
| CHANGELOG.md | Documentation of the new feature |
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Outdated
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Outdated
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Outdated
Show resolved
Hide resolved
...ation/src/test/java/com/azure/data/appconfiguration/implementation/QueryParamPolicyTest.java
Show resolved
Hide resolved
...ation/src/test/java/com/azure/data/appconfiguration/implementation/QueryParamPolicyTest.java
Outdated
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Outdated
Show resolved
Hide resolved
...ation/src/test/java/com/azure/data/appconfiguration/implementation/QueryParamPolicyTest.java
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
...ration/src/test/java/com/azure/data/appconfiguration/implementation/SyncTokenPolicyTest.java
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Outdated
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Outdated
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Outdated
Show resolved
Hide resolved
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Outdated
Show resolved
Hide resolved
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
...iguration/src/main/java/com/azure/data/appconfiguration/implementation/QueryParamPolicy.java
Show resolved
Hide resolved
|
What are all the azure monitor changes here? Do they relate to app config? |
Azure Monitor uses us to test there service. We broke there recordings. |
Description
Adds a new pipeline to update the query params to make sure they are all lowercase and in alphabetical order. This is required for support of Azure Front Door as a CDN.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines