Replace string ProxyAddress with IWebProxy for flexible proxy configuration#925
Merged
Conversation
…tibility Updates the README with an example
# Fixed conflicts: # README.md # Source/FikaAmazonAPI/AmazonCredential.cs
# Conflicts: # Source/FikaAmazonAPI/AmazonCredential.cs
Upgrades the proxy configuration from a plain string to IWebProxy, allowing callers to pass any proxy implementation including authenticated proxies via WebProxy credentials. A backwards-compatible string-based constructor overload is retained. RequestService simplified to a single code path; TokenGeneration extracts the address string for LWAClient which still requires one. README updated to reflect the new API.
Resolves conflicts in README.md, RequestService.cs, and TokenGeneration.cs from merging main into feature/WebProxy. LWAClient simplified to remove redundant null-proxy branch, consistent with RequestService. README wording updated to make clear the Proxy property accepts any IWebProxy implementation, with WebProxy shown as a concrete example only.
There was a problem hiding this comment.
Pull request overview
This PR updates proxy handling across the library by replacing the AmazonCredential.ProxyAddress string with an IWebProxy-based configuration, allowing richer proxy scenarios (e.g., credentials/authenticated proxies) and simplifying client creation code paths.
Changes:
- Replace
AmazonCredential.ProxyAddresswithAmazonCredential.Proxy(IWebProxy) and propagate the new type through token generation and LWA client construction. - Simplify
RequestServiceandLWAClientRest client setup to a single options path (proxy can be null). - Update README proxy configuration examples to use
IWebProxy/WebProxy.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Source/FikaAmazonAPI/Services/TokenGeneration.cs | Pass IWebProxy through to LWAClient during token refresh. |
| Source/FikaAmazonAPI/Services/RequestService.cs | Use RestClientOptions.Proxy directly from credentials. |
| Source/FikaAmazonAPI/AmazonSpApiSDK/Runtime/LWAClient.cs | Change proxy parameter to IWebProxy and simplify Rest client initialization. |
| Source/FikaAmazonAPI/AmazonCredential.cs | Replace proxy string property with IWebProxy and add a string-based constructor overload. |
| README.md | Document the new IWebProxy-based proxy configuration (including authenticated proxy example). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove duplicate using directive in LWAClient.cs - Rename var Client to var client in TokenGeneration.cs (camelCase) - Add [Obsolete] ProxyAddress property to preserve object-initialiser compatibility for existing callers - Remove the IWebProxy constructor overload to eliminate null ambiguity; callers should set the Proxy property directly - Rename constructor parameter to ProxyAddress to preserve named-argument compatibility - Use IsNullOrWhiteSpace instead of IsNullOrEmpty in proxy string handling
Collaborator
Author
|
All six Copilot review comments addressed in commit 045e70c:
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
string ProxyAddressproperty onAmazonCredentialwithIWebProxy Proxy, allowing callers to pass any proxy implementation (e.g. authenticated proxies viaWebProxycredentials, or customIWebProxyimplementations).ProxyAddressis retained as an[Obsolete]property that wrapsProxy, so existing object-initialiser callers continue to compile with a migration warningIWebProxyconstructor was considered but removed to avoidnull-argument ambiguity -- callers supplying anIWebProxydirectly should use theProxyproperty via object initialiser (new AmazonCredential { Proxy = ... })RequestServiceandLWAClient-- RestSharp handles a nullProxyvalue cleanly, so a single code path sufficesLWAClientupdated to acceptIWebProxydirectly rather than extracting a string address from itIWebProxyinterfaceTest plan
WebProxyaddress is providedWebProxywith credentials is providedProxyAddressvia object initialiser receive a deprecation warning but still compile