API updates for ClientCore based on review part 1 (Azure#44188)

API updates for ClientCore based on review part 1

Author: alzimmermsft

sdk/clientcore/annotation-processor/src/main/java/io/clientcore/annotation/processor/templating/JavaParserTemplateProcessor.java

@@ -36,7 +36,9 @@
 import javax.annotation.processing.ProcessingEnvironment;
 import java.io.IOException;
 import java.io.Writer;
+import java.lang.reflect.Field;
 import java.nio.ByteBuffer;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.stream.Collectors;
 
@@ -46,6 +48,29 @@
  * This class generates the implementation of the service interface.
  */
 public class JavaParserTemplateProcessor implements TemplateProcessor {
+    private static final Map<String, String> LOWERCASE_HEADER_TO_HTTPHEADENAME_CONSTANT;
+
+    static {
+        LOWERCASE_HEADER_TO_HTTPHEADENAME_CONSTANT = new HashMap<>();
+        for (Field field : HttpHeaderName.class.getDeclaredFields()) {
+            // Only inspect public static final fields (aka, constants)
+            if (!java.lang.reflect.Modifier.isPublic(field.getModifiers())
+                || !java.lang.reflect.Modifier.isStatic(field.getModifiers())
+                || !java.lang.reflect.Modifier.isFinal(field.getModifiers())) {
+                continue;
+            }
+
+            String constantName = field.getName();
+            HttpHeaderName httpHeaderName = null;
+            try {
+                httpHeaderName = (HttpHeaderName) field.get(null);
+            } catch (IllegalAccessException e) {
+                throw new RuntimeException(e);
+            }
+
+            LOWERCASE_HEADER_TO_HTTPHEADENAME_CONSTANT.put(httpHeaderName.getCaseInsensitiveName(), constantName);
+        }
+    }
 
     /**
      * Initializes a new instance of the {@link JavaParserTemplateProcessor} class.
@@ -268,8 +293,9 @@ private void initializeHttpRequest(BlockStmt body, HttpRequestContext method) {
         body.tryAddImportToParentCompilationUnit(HttpMethod.class);
 
         body.addStatement(StaticJavaParser.parseStatement("String host = " + method.getHost() + ";"));
-        Statement statement = StaticJavaParser.parseStatement(
-            "HttpRequest httpRequest = new HttpRequest(HttpMethod." + method.getHttpMethod() + ", host);");
+        Statement statement
+            = StaticJavaParser.parseStatement("HttpRequest httpRequest = new HttpRequest().setMethod(HttpMethod."
+                + method.getHttpMethod() + ").setUri(host);");
         statement.setLineComment("Create the HTTP request");
         body.addStatement(statement);
     }
@@ -282,35 +308,23 @@ private void addHeadersToRequest(BlockStmt body, HttpRequestContext method) {
         body.tryAddImportToParentCompilationUnit(HttpHeaders.class);
         body.tryAddImportToParentCompilationUnit(HttpHeaderName.class);
 
-        Statement statement = StaticJavaParser.parseStatement("HttpHeaders headers = new HttpHeaders();");
-        statement.setLineComment("Set the headers");
-        body.addStatement(statement);
         for (Map.Entry<String, String> header : method.getHeaders().entrySet()) {
-            String enumHeaderKey = header.getKey().toUpperCase().replace("-", "_");
-            boolean isEnumExists = false;
-            for (HttpHeaderName httpHeaderName : HttpHeaderName.values()) {
-                if (httpHeaderName.getCaseInsensitiveName().equals(header.getKey().toLowerCase())) {
-                    isEnumExists = true;
-                    break;
-                }
-            }
-
             boolean isStringType = method.getParameters()
                 .stream()
                 .anyMatch(parameter -> parameter.getName().equals(header.getValue())
                     && "String".equals(parameter.getShortTypeName()));
             String value = isStringType ? header.getValue() : "String.valueOf(" + header.getValue() + ")";
 
-            if (isEnumExists) {
-                body.addStatement(StaticJavaParser
-                    .parseStatement("headers.add(HttpHeaderName." + enumHeaderKey + ", " + value + ");"));
-            } else {
+            String constantName = LOWERCASE_HEADER_TO_HTTPHEADENAME_CONSTANT.get(header.getKey().toLowerCase());
+            if (constantName != null) {
                 body.addStatement(StaticJavaParser.parseStatement(
-                    "headers.add(HttpHeaderName.fromString(\"" + header.getKey() + "\"), " + value + ");"));
+                    "httpRequest.getHeaders().add(HttpHeaderName." + constantName + ", " + value + ");"));
+            } else {
+                body.addStatement(
+                    StaticJavaParser.parseStatement("httpRequest.getHeaders().add(HttpHeaderName.fromString(\""
+                        + header.getKey() + "\"), " + value + ");"));
             }
         }
-
-        body.addStatement(StaticJavaParser.parseStatement("httpRequest.setHeaders(headers);"));
     }
 
     private void addRequestBody(BlockStmt body, HttpRequestContext method) {

sdk/clientcore/annotation-processor/src/test/java/io/clientcore/annotation/processor/models/TemplateInputTest.java

@@ -97,11 +97,6 @@ void setAndGetUnexpectedResponseExceptionDetails() {
     }
 
     private static final class MockUnexpectedResponseExceptionDetail implements UnexpectedResponseExceptionDetail {
-        @Override
-        public String exceptionTypeName() {
-            return "MockException";
-        }
-
         @Override
         public int[] statusCode() {
             return new int[0];

sdk/clientcore/core/checkstyle-suppressions.xml

@@ -19,17 +19,16 @@
   <suppress files="io.clientcore.core.implementation.utils.ImplUtils.java" checks="MissingJavadocTypeCheck" />
   <suppress files="io.clientcore.core.implementation.instrumentation.Slf4jLoggerShim.java" checks="com.azure.tools.checkstyle.checks.EnforceFinalFieldsCheck" />
   <suppress files="io.clientcore.core.implementation.utils.EnvironmentConfiguration.java" checks="com.azure.tools.checkstyle.checks.EnforceFinalFieldsCheck" />
-  <suppress files="io.clientcore.core.utils.Union.java" checks="com.azure.tools.checkstyle.checks.EnforceFinalFieldsCheck" />
   <suppress files="io.clientcore.core.implementation.ReflectionSerializable.java" checks="com.azure.tools.checkstyle.checks.JavadocThrowsChecks" />
   <suppress files="io.clientcore.core.implementation.http.serializer.HttpResponseBodyDecoder.java" checks="com.azure.tools.checkstyle.checks.JavadocThrowsChecks" />
-  <suppress files="io.clientcore.core.http.client.DefaultHttpClientBuilder.java" checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" />
+  <suppress files="io.clientcore.core.http.client.JdkHttpClientBuilder.java" checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" />
   <suppress files="io.clientcore.core.http.pipeline.HttpInstrumentationPolicy.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
   <suppress files="io.clientcore.core.implementation.MethodHandleReflectiveInvoker.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
   <suppress files="io.clientcore.core.implementation.http.rest.LengthValidatingInputStream.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
+  <suppress files="io.clientcore.core.instrumentation.logging.LoggingEvent.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
   <suppress files="io.clientcore.core.serialization.json.JsonReader.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
   <suppress files="io.clientcore.core.serialization.json.JsonWriteContext.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
-  <suppress files="io.clientcore.core.serialization.json.implementation.DefaultJsonReader.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
-  <suppress files="io.clientcore.core.serialization.json.implementation.DefaultJsonWriter.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
+  <suppress files="io.clientcore.core.serialization.json.JsonWriter.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
   <suppress files="io.clientcore.core.serialization.json.implementation.StringBuilderWriter.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
   <suppress files="io.clientcore.core.serialization.json.models.JsonNumber.java" checks="com.azure.tools.checkstyle.checks.UseCaughtExceptionCauseCheck" />
 </suppressions>

sdk/clientcore/core/spotbugs-exclude.xml

@@ -18,6 +18,7 @@
       <Class name="io.clientcore.core.http.pipeline.HttpPipelineBuilder" />
       <Class name="io.clientcore.core.http.pipeline.HttpPipelineNextPolicy" />
       <Class name="io.clientcore.core.http.pipeline.HttpRetryPolicy" />
+      <Class name="io.clientcore.core.implementation.GenericParameterizedType" />
       <Class name="io.clientcore.core.implementation.ReflectionUtilsMethodHandle" />
       <Class name="io.clientcore.core.implementation.http.HttpPipelineCallState" />
       <Class name="io.clientcore.core.implementation.http.rest.PercentEscaper" />
@@ -27,8 +28,6 @@
       <Class name="io.clientcore.core.implementation.http.rest.ResponseConstructorsNoCacheReflection" />
       <Class name="io.clientcore.core.implementation.http.rest.ResponseExceptionConstructorCache" />
       <Class name="io.clientcore.core.implementation.http.rest.RestProxyImpl" />
-      <Class name="io.clientcore.core.implementation.http.serializer.CompositeSerializer" />
-      <Class name="io.clientcore.core.utils.DateTimeRfc1123" />
       <Class name="io.clientcore.core.implementation.utils.InternalContext" />
       <Class name="io.clientcore.core.implementation.utils.JsonSerializer" />
       <Class name="io.clientcore.core.implementation.utils.Providers" />
@@ -37,7 +36,7 @@
       <Class name="io.clientcore.core.implementation.utils.XmlSerializer" />
       <Class name="io.clientcore.core.serialization.xml.XmlReader" />
       <Class name="io.clientcore.core.shared.HttpClientTests" />
-      <Class name="io.clientcore.core.implementation.GenericParameterizedType" />
+      <Class name="io.clientcore.core.utils.DateTimeRfc1123" />
       <Class name="io.clientcore.core.utils.SharedExecutorService" />
       <Class name="io.clientcore.core.utils.Union" />
       <Class name="io.clientcore.core.utils.binarydata.FileBinaryData" />
@@ -46,15 +45,11 @@
       <Class name="io.clientcore.core.utils.configuration.Configuration" />
     </Or>
   </Match>
-  <Match>
-    <Bug pattern="BC_VACUOUS_INSTANCEOF" />
-    <Class name="io.clientcore.core.http.pipeline.HttpRetryPolicy" />
-  </Match>
   <Match>
     <Bug pattern="CNT_ROUGH_CONSTANT_VALUE" />
     <Or>
       <Class name="io.clientcore.core.implementation.serializer.BinaryDataSerializationTests" />
-      <Class name="io.clientcore.core.serialization.json.contract.JsonReaderContractTests" />
+      <Class name="io.clientcore.core.serialization.json.JsonReaderTests" />
     </Or>
   </Match>
   <Match>
@@ -69,9 +64,7 @@
     <Bug pattern="CT_CONSTRUCTOR_THROW" />
     <Or>
       <Class name="io.clientcore.core.credentials.KeyCredential" />
-      <Class name="io.clientcore.core.http.client.DefaultHttpClient" />
-      <Class name="io.clientcore.core.http.client.DefaultHttpClientBuilder" />
-      <Class name="io.clientcore.core.http.models.HttpRequest" />
+      <Class name="io.clientcore.core.http.client.JdkHttpClientBuilder" />
       <Class name="io.clientcore.core.http.pipeline.KeyCredentialPolicy" />
       <Class name="io.clientcore.core.implementation.http.rest.SwaggerMethodParser" />
       <Class name="io.clientcore.core.implementation.instrumentation.otel.OTelInstrumentation" />
@@ -93,17 +86,12 @@
   </Match>
   <Match>
     <Bug pattern="DMI_RANDOM_USED_ONLY_ONCE" />
-    <Or>
-      <Class name="io.clientcore.core.http.pipeline.HttpPipelinePolicyTests$SyncPolicy" />
-      <Class name="io.clientcore.core.utils.TestUtils" />
-    </Or>
+    <Class name="io.clientcore.core.utils.TestUtils" />
   </Match>
   <Match>
     <Bug pattern="DM_CONVERT_CASE" />
     <Or>
-      <Class name="io.clientcore.core.http.exceptions.HttpExceptionType" />
       <Class name="io.clientcore.core.http.models.HttpHeaderName" />
-      <Class name="io.clientcore.core.http.pipeline.HttpPipelineBuilder" />
       <Class name="io.clientcore.core.utils.ServerSentEventUtils" />
       <Class name="io.clientcore.core.utils.auth.AuthUtils" />
     </Or>
@@ -112,9 +100,8 @@
     <Bug pattern="DM_DEFAULT_ENCODING" />
     <Or>
       <Class name="io.clientcore.core.http.RestProxyTests" />
-      <Class name="io.clientcore.core.http.client.DefaultHttpClientIT" />
       <Class name="io.clientcore.core.http.client.SimpleBasicAuthHttpProxyServer" />
-      <Class name="io.clientcore.core.http.pipeline.HttpPipelinePolicyTests$SyncPolicy" />
+      <Class name="io.clientcore.core.implementation.http.client.DefaultHttpClientIT" />
       <Class name="io.clientcore.core.implementation.http.rest.RestProxyImplTests" />
       <Class name="io.clientcore.core.implementation.serializer.AdditionalPropertiesSerializerTests" />
       <Class name="io.clientcore.core.implementation.serializer.BinaryDataSerializationTests" />
@@ -124,8 +111,8 @@
       <Class name="io.clientcore.core.implementation.serializer.JsonSerializableEndToEndTests" />
       <Class name="io.clientcore.core.instrumentation.logging.ClientLoggerTests" />
       <Class name="io.clientcore.core.instrumentation.logging.InstrumentationTestUtils" />
-      <Class name="io.clientcore.core.serialization.json.implementation.DefaultJsonReader" />
-      <Class name="io.clientcore.core.serialization.json.implementation.DefaultJsonWriter" />
+      <Class name="io.clientcore.core.serialization.json.JsonReader" />
+      <Class name="io.clientcore.core.serialization.json.JsonWriter" />
       <Class name="io.clientcore.core.serialization.xml.XmlReader" />
       <Class name="io.clientcore.core.shared.HttpClientTests" />
       <Class name="io.clientcore.core.shared.HttpClientTestsServer" />
@@ -154,7 +141,7 @@
   </Match>
   <Match>
     <Bug pattern="ES_COMPARING_STRINGS_WITH_EQ" />
-    <Class name="io.clientcore.core.serialization.json.contract.models.JsonStringContractTests" />
+    <Class name="io.clientcore.core.serialization.json.models.JsonStringTests" />
   </Match>
   <Match>
     <Bug pattern="HARD_CODE_PASSWORD" />
@@ -236,10 +223,9 @@
   <Match>
     <Bug pattern="NP_NULL_PARAM_DEREF_ALL_TARGETS_DANGEROUS" />
     <Or>
-      <Class name="io.clientcore.core.serialization.json.contract.JsonWriterContractTests" />
       <Class name="io.clientcore.core.implementation.instrumentation.fallback.FallbackInstrumentationTests" />
+      <Class name="io.clientcore.core.serialization.json.JsonWriterTests" />
     </Or>
-
   </Match>
   <Match>
     <Bug pattern="NP_NULL_PARAM_DEREF_NONVIRTUAL" />
@@ -273,10 +259,10 @@
   <Match>
     <Bug pattern="PZLA_PREFER_ZERO_LENGTH_ARRAYS" />
     <Or>
-      <Class name="io.clientcore.core.utils.Base64Uri" />
       <Class name="io.clientcore.core.implementation.utils.JsonSerializer" />
       <Class name="io.clientcore.core.implementation.utils.XmlSerializer" />
-      <Class name="io.clientcore.core.serialization.json.implementation.DefaultJsonReader" />
+      <Class name="io.clientcore.core.serialization.json.JsonReader" />
+      <Class name="io.clientcore.core.utils.Base64Uri" />
       <Class name="io.clientcore.core.utils.TestUtils" />
       <Class name="io.clientcore.core.utils.serializers.MockSerializer" />
     </Or>
@@ -291,11 +277,7 @@
   </Match>
   <Match>
     <Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
-    <Class name="io.clientcore.core.http.client.implementation.InputStreamTimeoutResponseSubscriber" />
-  </Match>
-  <Match>
-    <Bug pattern="RV_RETURN_VALUE_IGNORED_INFERRED" />
-    <Class name="io.clientcore.core.utils.UnionTests" />
+    <Class name="io.clientcore.core.implementation.http.client.InputStreamTimeoutResponseSubscriber" />
   </Match>
   <Match>
     <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
@@ -311,10 +293,7 @@
   </Match>
   <Match>
     <Bug pattern="SE_BAD_FIELD" />
-    <Or>
-      <Class name="io.clientcore.core.http.exceptions.HttpResponseException" />
-      <Class name="io.clientcore.core.serialization.json.implementation.jackson.core.util.RequestPayload" />
-    </Or>
+    <Class name="io.clientcore.core.serialization.json.implementation.jackson.core.util.RequestPayload" />
   </Match>
   <Match>
     <Bug pattern="SE_NO_SERIALVERSIONID" />
@@ -347,7 +326,6 @@
   <Match>
     <Bug pattern="SIC_INNER_SHOULD_BE_STATIC_ANON" />
     <Or>
-      <Class name="io.clientcore.core.http.pipeline.HttpPipelinePolicyTests" />
       <Class name="io.clientcore.core.http.pipeline.HttpPipelineTests" />
       <Class name="io.clientcore.core.http.pipeline.RedirectPolicyTest" />
       <Class name="io.clientcore.core.http.pipeline.RetryPolicyTests" />
@@ -370,9 +348,8 @@
   <Match>
     <Bug pattern="UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR" />
     <Or>
-      <Class name="io.clientcore.core.http.client.implementation.InputStreamTimeoutResponseSubscriber" />
+      <Class name="io.clientcore.core.implementation.http.client.InputStreamTimeoutResponseSubscriber" />
       <Class name="io.clientcore.core.implementation.http.rest.ResponseConstructorsCacheBenchMark" />
-      <Class name="io.clientcore.core.serialization.json.implementation.DefaultJsonWriterContractTests" />
       <Class name="io.clientcore.core.serialization.xml.DefaultXmlWriterContractTests" />
       <Class name="io.clientcore.core.serialization.xml.implementation.aalto.in.ReaderConfig$EncodingContext" />
       <Class name="io.clientcore.core.utils.binarydata.BinaryDataTest$BinaryDataAsProperty" />
@@ -407,7 +384,7 @@
   </Match>
   <Match>
     <Bug pattern="WMI_WRONG_MAP_ITERATOR" />
-    <Class name="io.clientcore.core.serialization.json.contract.JsonReaderContractTests" />
+    <Class name="io.clientcore.core.serialization.json.JsonReaderTests" />
   </Match>
   <Match>
     <Bug pattern="XXE_XMLSTREAMREADER" />

sdk/clientcore/core/src/main/java/io/clientcore/core/http/annotations/UnexpectedResponseExceptionDetail.java

@@ -3,7 +3,6 @@
 
 package io.clientcore.core.http.annotations;
 
-import io.clientcore.core.http.exceptions.HttpExceptionType;
 import io.clientcore.core.http.exceptions.HttpResponseException;
 
 import java.lang.annotation.Repeatable;
@@ -14,9 +13,8 @@
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
 /**
- * The {@link HttpExceptionType} that is thrown or returned when one of the status codes is returned from a REST API. Multiple
- * annotations can be used. When no codes are listed that exception is always thrown or returned if it is reached
- * during evaluation, this should be treated as a default case. If no default case is annotated the fall through
+ * Multiple annotations can be used. When no codes are listed that exception is always thrown or returned if it is
+ * reached during evaluation, this should be treated as a default case. If no default case is annotated the fall through
  * exception is {@link HttpResponseException}.
  *
  * <p><strong>Example:</strong></p>
@@ -56,14 +54,6 @@
 @Target(METHOD)
 @Repeatable(UnexpectedResponseExceptionDetails.class)
 public @interface UnexpectedResponseExceptionDetail {
-    /**
-     * The name of the {@link HttpExceptionType} of an {@link HttpResponseException} that should be thrown/returned when
-     * the API returns an unrecognized status code.
-     *
-     * @return The {@link HttpExceptionType} that should be thrown/returned, represented as a {@link String}.
-     */
-    String exceptionTypeName() default "";
-
     /**
      * HTTP status codes which trigger the {@link HttpResponseException} to be thrown or returned. If no status codes
      * are listed the exception is always thrown or returned.