-
Notifications
You must be signed in to change notification settings - Fork 965
Add tests to override configuration with plugins #4572
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
Add tests to override configuration with plugins #4572
Conversation
| import software.amazon.awssdk.profiles.ProfileFileSupplier; | ||
|
|
||
| @SdkInternalApi | ||
| public final class SdkClientConfigurationUtil { |
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.
Renamed from SdkClientConfigurationUtil.java.resource to SdkClientConfigurationUtil.resource.java to make it easier for editors to identify it as a Java file. Also removed the method copyConfigurationToOverrides which is not longer used.
feb6701 to
823b307
Compare
|
|
||
| // Assert that we can retrieve the same value | ||
| assertThat(testCase.getter.apply(anotherBuilder)).isEqualTo(testCase.value); | ||
| if (testCase.hasDirectMapping) { |
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.
We no longer "mirror" the client override configuration from the client configuration, therefore this test no longer works for those properties.
823b307 to
f4e0ae5
Compare
|
|
||
| <!-- See SpotBugs bug: https://github.com/spotbugs/spotbugs/issues/600, https://github.com/spotbugs/spotbugs/issues/756 --> | ||
| <Match> | ||
| <Class name="software.amazon.awssdk.core.SdkServiceClientConfiguration$Builder"/> |
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.
SpotBugs gets confused about the nullability of the overrideConfiguration, probably because is looking at the default method that throws an UnsupportedOperationException. Adding this since I'm certain that this value can be null.
|
SonarCloud Quality Gate failed.
|












Motivation and Context
Changes to make sure that all overrides using the client builder can also be applied using plugins.
Modifications
This PR changes the way we deal with the
ClientOverrideConfiguration. Instead of rebuilding it from its side effect (i.e., the values inSdkClientConfiguration) we use the override configuration object that is present during client building or at request execution time. This will reflect the exact values given for this object without having any "magic" derived values to appear. For instance, the execution interceptors in the override configuration are added to the same client optionEXECUTION_INTERCEPTORSbut this is also filled up with default interceptors here but those are not part of the original override configuration but before this change would be present in theSdkServiceClientConfiguration.Builderobject since those were copied blindly.We also made changes to the way we set the values to avoid overwriting previously added or computed ones, this means the if a list is given (e.g., with execution interceptors) we will follow a conservative logic to avoid duplicating previously added or removing computed values. For instance:
Testing
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License