Revert "Make execution attributes immutable and fix other PR comments"

This reverts commit ad9a6d3.

Author: dagnir

core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/handler/AwsClientHandlerUtils.java

@@ -40,7 +40,11 @@
 import software.amazon.awssdk.core.client.config.SdkClientOption;
 import software.amazon.awssdk.core.client.handler.ClientExecutionParams;
 import software.amazon.awssdk.core.http.ExecutionContext;
-import software.amazon.awssdk.core.interceptor.*;
+import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
+import software.amazon.awssdk.core.interceptor.ExecutionInterceptorChain;
+import software.amazon.awssdk.core.interceptor.InterceptorContext;
+import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute;
+import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute;
 import software.amazon.awssdk.core.metrics.CoreMetric;
 import software.amazon.awssdk.core.signer.Signer;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
@@ -102,7 +106,8 @@ static <InputT extends SdkRequest, OutputT extends SdkResponse> ExecutionContext
             executionAttributes = clientOverrideExecutionAttributes.merge(executionAttributes);
         }
 
-        Optional<? extends RequestOverrideConfiguration> requestOverrideConfiguration = originalRequest.overrideConfiguration();
+        Optional<RequestOverrideConfiguration> requestOverrideConfiguration =
+                (Optional<RequestOverrideConfiguration>) originalRequest.overrideConfiguration();
         if (requestOverrideConfiguration.isPresent()) {
             executionAttributes = requestOverrideConfiguration.get().executionAttributes().merge(executionAttributes);
         }

core/sdk-core/src/main/java/software/amazon/awssdk/core/RequestOverrideConfiguration.java

@@ -29,7 +29,6 @@
 import software.amazon.awssdk.annotations.SdkPublicApi;
 import software.amazon.awssdk.core.interceptor.ExecutionAttribute;
 import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
-import software.amazon.awssdk.core.interceptor.UnmodifiableExecutionAttributes;
 import software.amazon.awssdk.core.signer.Signer;
 import software.amazon.awssdk.metrics.MetricPublisher;
 import software.amazon.awssdk.utils.CollectionUtils;
@@ -59,7 +58,7 @@ protected RequestOverrideConfiguration(Builder<?> builder) {
         this.apiCallAttemptTimeout = Validate.isPositiveOrNull(builder.apiCallAttemptTimeout(), "apiCallAttemptTimeout");
         this.signer = builder.signer();
         this.metricPublishers = Collections.unmodifiableList(new ArrayList<>(builder.metricPublishers()));
-        this.executionAttributes = new UnmodifiableExecutionAttributes(builder.executionAttributes().build());
+        this.executionAttributes = builder.executionAttributes().build();
     }
 
     /**

core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java

@@ -28,7 +28,6 @@
 import software.amazon.awssdk.core.interceptor.ExecutionAttribute;
 import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
 import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
-import software.amazon.awssdk.core.interceptor.UnmodifiableExecutionAttributes;
 import software.amazon.awssdk.core.retry.RetryMode;
 import software.amazon.awssdk.core.retry.RetryPolicy;
 import software.amazon.awssdk.core.sync.ResponseTransformer;
@@ -75,7 +74,7 @@ private ClientOverrideConfiguration(Builder builder) {
         this.defaultProfileFile = builder.defaultProfileFile();
         this.defaultProfileName = builder.defaultProfileName();
         this.metricPublishers = Collections.unmodifiableList(new ArrayList<>(builder.metricPublishers()));
-        this.executionAttributes = new UnmodifiableExecutionAttributes(builder.executionAttributes().build());
+        this.executionAttributes = builder.executionAttributes().build();
     }
 
     @Override

core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/ExecutionAttribute.java

@@ -16,9 +16,6 @@
 package software.amazon.awssdk.core.interceptor;
 
 import software.amazon.awssdk.annotations.SdkPublicApi;
-import software.amazon.awssdk.core.RequestOverrideConfiguration;
-
-import java.util.Objects;
 
 /**
  * An attribute attached to a particular execution, stored in {@link ExecutionAttributes}.
@@ -60,32 +57,4 @@ public ExecutionAttribute(String name) {
     public String toString() {
         return name;
     }
-
-    /**
-     * This override considers execution attributes with the same name
-     * to be the same object for the purpose of attribute merge.
-     * @return boolean indicating whether the objects are equal or not.
-     */
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) {
-            return true;
-        }
-        if (o == null || getClass() != o.getClass()) {
-            return false;
-        }
-
-        ExecutionAttribute that = (ExecutionAttribute) o;
-        return that.name.equals(this.name);
-    }
-
-    /**
-     * This override considers execution attributes with the same name
-     * to be the same object for the purpose of attribute merge.
-     * @return hash code
-     */
-    @Override
-    public int hashCode() {
-        return Objects.hashCode(name);
-    }
 }

core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/ExecutionAttributes.java

@@ -31,14 +31,14 @@
  */
 @SdkPublicApi
 @NotThreadSafe
-public class ExecutionAttributes implements ToCopyableBuilder<ExecutionAttributes.Builder, ExecutionAttributes> {
+public final class ExecutionAttributes implements ToCopyableBuilder<ExecutionAttributes.Builder, ExecutionAttributes> {
     private final Map<ExecutionAttribute<?>, Object> attributes;
 
     public ExecutionAttributes() {
         this.attributes = new HashMap<>();
     }
 
-    protected ExecutionAttributes(Map<? extends ExecutionAttribute<?>, ?> attributes) {
+    private ExecutionAttributes(Map<? extends ExecutionAttribute<?>, ?> attributes) {
         this.attributes = new HashMap<>(attributes);
     }
 
@@ -66,13 +66,20 @@ public <U> ExecutionAttributes putAttribute(ExecutionAttribute<U> attribute, U v
         return this;
     }
 
+    /**
+     * Add all execution attributes from the provided collection to the internal collection of attributes.
+     */
+    public <U> ExecutionAttributes putAllAttributes(Map<ExecutionAttribute<?>, ?> executionAttributes) {
+        this.attributes.putAll(executionAttributes);
+        return this;
+    }
+
     /**
     * Merge attributes of a lower precedence into the current higher precedence current collection.
     */
-    public ExecutionAttributes merge(ExecutionAttributes lowerPrecedenceExecutionAttributes) {
-        Map<ExecutionAttribute<?>, Object> copiedAttributes = new HashMap<>(this.attributes);
-        lowerPrecedenceExecutionAttributes.getAttributes().forEach(copiedAttributes::putIfAbsent);
-        return new ExecutionAttributes(copiedAttributes);
+    public <U> ExecutionAttributes merge(ExecutionAttributes lowerPrecedenceExecutionAttributes) {
+        lowerPrecedenceExecutionAttributes.getAttributes().forEach(this.attributes::putIfAbsent);
+        return this;
     }
 
     /**
@@ -103,7 +110,7 @@ public static final class Builder implements CopyableBuilder<ExecutionAttributes
         private Builder() {
         }
 
-
+        @SuppressWarnings("unchecked") // Cast is safe due to implementation of {@link #put}
         public <T> T get(ExecutionAttribute<T> key) {
             Validate.notNull(key, "Key to retrieve must not be null.");
             return (T) executionAttributes.get(key);

core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/UnmodifiableExecutionAttributes.java

@@ -17,7 +17,6 @@
 
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 
 import java.util.ArrayList;
@@ -27,7 +26,6 @@
 import java.util.List;
 import java.util.Map;
 import org.junit.Test;
-import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
 import software.amazon.awssdk.core.interceptor.ExecutionAttribute;
 import software.amazon.awssdk.metrics.MetricPublisher;
 import software.amazon.awssdk.utils.ImmutableMap;
@@ -185,25 +183,6 @@ public void executionAttributes_createsCopy() {
         assertThat(overrideConfig.executionAttributes().getAttribute(testAttribute)).isEqualTo(expectedValue);
     }
 
-    @Test
-    public void executionAttributes_isImmutable() {
-        Map<ExecutionAttribute<?>, Object> executionAttributes = new HashMap<>();
-
-        ExecutionAttribute testAttribute = new ExecutionAttribute("TestAttribute");
-        String expectedValue = "Value1";
-        executionAttributes.put(testAttribute, expectedValue);
-
-        SdkRequestOverrideConfiguration overrideConfig = SdkRequestOverrideConfiguration.builder()
-                .executionAttributes(executionAttributes)
-                .build();
-        try {
-            overrideConfig.executionAttributes().putAttribute(testAttribute, 2);
-            fail("Expected unsupported operation exception");
-        } catch(Exception ex) {
-            assertThat(ex instanceof UnsupportedOperationException).isTrue();
-        }
-    }
-
     @Test
     public void executionAttributes_maintainsAllAdded() {
         Map<ExecutionAttribute, Object> executionAttributeObjectMap = new HashMap<>();

core/sdk-core/src/test/java/software/amazon/awssdk/core/RequestOverrideConfigurationTest.java

@@ -16,7 +16,8 @@
 package software.amazon.awssdk.core.client.config;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 
 import java.util.ArrayList;
@@ -197,26 +198,6 @@ public void executionAttributes_createsCopy() {
         assertThat(overrideConfig.executionAttributes().getAttribute(testAttribute)).isEqualTo(expectedValue);
     }
 
-    @Test
-    public void executionAttributes_isImmutable() {
-        Map<ExecutionAttribute<?>, Object> executionAttributes = new HashMap<>();
-
-        ExecutionAttribute testAttribute = new ExecutionAttribute("TestAttribute");
-        String expectedValue = "Value1";
-        executionAttributes.put(testAttribute, expectedValue);
-
-        ClientOverrideConfiguration overrideConfig = ClientOverrideConfiguration.builder()
-                .executionAttributes(executionAttributes)
-                .build();
-
-        try {
-            overrideConfig.executionAttributes().putAttribute(testAttribute, 2);
-            fail("Expected unsupported operation exception");
-        } catch(Exception ex) {
-            assertThat(ex instanceof UnsupportedOperationException).isTrue();
-        }
-    }
-
     @Test
     public void executionAttributes_maintainsAllAdded() {
         Map<ExecutionAttribute, Object> executionAttributeObjectMap = new HashMap<>();

core/sdk-core/src/test/java/software/amazon/awssdk/core/client/config/ClientOverrideConfigurationTest.java

@@ -149,29 +149,4 @@ public void asyncClient_requestOverrideExecutionAttributesHavePrecedence() {
             async.close();
         }
     }
-
-    @Test
-    public void testExecutionAttributesMerge() {
-        ExecutionAttributes lowerPrecedence = new ExecutionAttributes();
-        for (int i = 0; i < 3; i++) {
-            lowerPrecedence.putAttribute(getAttribute(i), 1);
-        }
-
-        ExecutionAttributes higherPrecendence = new ExecutionAttributes();
-        for (int i = 2; i < 4; i++) {
-            higherPrecendence.putAttribute(getAttribute(i), 2);
-        }
-
-        ExecutionAttributes expectedAttributes = new ExecutionAttributes();
-        for (int i = 0; i < 4; i++) {
-            expectedAttributes.putAttribute(getAttribute(i),  i >= 2 ? 2 : 1);
-        }
-
-        ExecutionAttributes mergedAttributes = higherPrecendence.merge(lowerPrecedence);
-        assertThat(mergedAttributes.getAttributes()).isEqualTo(expectedAttributes.getAttributes());
-    }
-
-    private ExecutionAttribute getAttribute(int i) {
-        return new ExecutionAttribute<>("TestAttribute" + i);
-    }
 }