chore: Code Cleanup Before GA Release (#212)

* Some code cleanup

* Remove Spring Dependency

* Refactor Jackson into central mapper config

* Minor update

* Revert HTML changes

* Move Jackson to separate class

* Remove redundant javadoc

Author: MatKuhr

core/pom.xml

@@ -86,6 +86,10 @@
       <groupId>com.fasterxml.jackson.datatype</groupId>
       <artifactId>jackson-datatype-jsr310</artifactId>
     </dependency>
+    <dependency>
+      <groupId>com.fasterxml.jackson.module</groupId>
+      <artifactId>jackson-module-parameter-names</artifactId>
+    </dependency>
     <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>

core/src/main/java/com/sap/ai/sdk/core/AiCoreService.java

@@ -1,9 +1,8 @@
 package com.sap.ai.sdk.core;
 
-import com.fasterxml.jackson.annotation.JsonAutoDetect;
-import com.fasterxml.jackson.annotation.JsonInclude;
-import com.fasterxml.jackson.annotation.PropertyAccessor;
-import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+import static com.sap.ai.sdk.core.JacksonConfiguration.getDefaultObjectMapper;
+
+import com.fasterxml.jackson.databind.json.JsonMapper;
 import com.google.common.annotations.Beta;
 import com.google.common.collect.Iterables;
 import com.sap.cloud.sdk.cloudplatform.connectivity.ApacheHttpClient5Accessor;
@@ -21,7 +20,6 @@
 import lombok.val;
 import org.springframework.http.client.BufferingClientHttpRequestFactory;
 import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
-import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder;
 import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
 import org.springframework.web.client.RestTemplate;
 
@@ -37,6 +35,7 @@ public class AiCoreService {
   /** The default resource group. */
   public static final String DEFAULT_RESOURCE_GROUP = "default";
 
+  private static final JsonMapper objectMapper = getDefaultObjectMapper();
   private static final String RESOURCE_GROUP_HEADER_PROPERTY = "URL.headers.AI-Resource-Group";
 
   @Nonnull private final Supplier<HttpDestination> baseDestinationResolver;
@@ -131,14 +130,6 @@ public ApiClient getApiClient() {
     httpRequestFactory.setHttpClient(ApacheHttpClient5Accessor.getHttpClient(destination));
 
     val rt = new RestTemplate();
-    val objectMapper =
-        new Jackson2ObjectMapperBuilder()
-            .modules(new JavaTimeModule())
-            .visibility(PropertyAccessor.GETTER, JsonAutoDetect.Visibility.NONE)
-            .visibility(PropertyAccessor.SETTER, JsonAutoDetect.Visibility.NONE)
-            .serializationInclusion(JsonInclude.Include.NON_NULL) // THIS STOPS `null` serialization
-            .build();
-
     Iterables.filter(rt.getMessageConverters(), MappingJackson2HttpMessageConverter.class)
         .forEach(converter -> converter.setObjectMapper(objectMapper));
     rt.setRequestFactory(new BufferingClientHttpRequestFactory(httpRequestFactory));

core/src/main/java/com/sap/ai/sdk/core/JacksonConfiguration.java

@@ -0,0 +1,45 @@
+package com.sap.ai.sdk.core;
+
+import static com.fasterxml.jackson.databind.MapperFeature.DEFAULT_VIEW_INCLUSION;
+
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.json.JsonMapper;
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+import com.fasterxml.jackson.module.paramnames.ParameterNamesModule;
+import com.google.common.annotations.Beta;
+import javax.annotation.Nonnull;
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+
+/** Internal utility class for getting a default object mapper with preset configuration. */
+@NoArgsConstructor(access = AccessLevel.NONE)
+public final class JacksonConfiguration {
+
+  /**
+   * Default object mapper used for JSON de-/serialization. <b>Only intended for internal usage
+   * within this SDK</b>. Largely follows the defaults set by Spring.
+   *
+   * @return A new object mapper with the default configuration.
+   * @see <a
+   *     href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.html">Jackson2ObjectMapperBuilder</a>
+   */
+  @Nonnull
+  @Beta
+  public static JsonMapper getDefaultObjectMapper() {
+    return JsonMapper.builder()
+        .addModule(new JavaTimeModule())
+        .addModule(new ParameterNamesModule())
+        // Disable automatic detection of getters and setters
+        .visibility(PropertyAccessor.GETTER, JsonAutoDetect.Visibility.NONE)
+        .visibility(PropertyAccessor.SETTER, JsonAutoDetect.Visibility.NONE)
+        .serializationInclusion(JsonInclude.Include.NON_NULL)
+        // for generated code unknown properties should always be stored as custom fields
+        // still added for any non-generated code and additional safety
+        .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
+        .disable(DEFAULT_VIEW_INCLUSION)
+        .build();
+  }
+}

foundation-models/openai/pom.xml

@@ -35,10 +35,6 @@
     <project.rootdir>${project.basedir}/../../</project.rootdir>
   </properties>
   <dependencies>
-    <dependency>
-      <groupId>org.springframework</groupId>
-      <artifactId>spring-web</artifactId>
-    </dependency>
     <dependency>
       <groupId>com.sap.cloud.sdk.cloudplatform</groupId>
       <artifactId>cloudplatform-connectivity</artifactId>
@@ -75,10 +71,6 @@
       <groupId>com.fasterxml.jackson.core</groupId>
       <artifactId>jackson-annotations</artifactId>
     </dependency>
-    <dependency>
-      <groupId>com.fasterxml.jackson.datatype</groupId>
-      <artifactId>jackson-datatype-jsr310</artifactId>
-    </dependency>
     <dependency>
       <groupId>io.vavr</groupId>
       <artifactId>vavr</artifactId>

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClient.java

@@ -1,11 +1,9 @@
 package com.sap.ai.sdk.foundationmodels.openai;
 
-import com.fasterxml.jackson.annotation.JsonAutoDetect;
-import com.fasterxml.jackson.annotation.JsonInclude;
-import com.fasterxml.jackson.annotation.PropertyAccessor;
+import static com.sap.ai.sdk.core.JacksonConfiguration.getDefaultObjectMapper;
+
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
 import com.google.common.annotations.Beta;
 import com.sap.ai.sdk.core.AiCoreService;
 import com.sap.ai.sdk.core.DeploymentResolutionException;
@@ -24,32 +22,22 @@
 import java.io.IOException;
 import java.util.stream.Stream;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import lombok.AccessLevel;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.hc.client5.http.classic.methods.HttpPost;
 import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.io.entity.StringEntity;
 import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
-import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder;
 
 /** Client for interacting with OpenAI models. */
 @Slf4j
 @RequiredArgsConstructor(access = AccessLevel.PRIVATE)
 public final class OpenAiClient {
   private static final String DEFAULT_API_VERSION = "2024-02-01";
-  static final ObjectMapper JACKSON;
-  private String systemPrompt = null;
-
-  static {
-    JACKSON =
-        new Jackson2ObjectMapperBuilder()
-            .modules(new JavaTimeModule())
-            .visibility(PropertyAccessor.GETTER, JsonAutoDetect.Visibility.NONE)
-            .visibility(PropertyAccessor.SETTER, JsonAutoDetect.Visibility.NONE)
-            .serializationInclusion(JsonInclude.Include.NON_NULL)
-            .build();
-  }
+  static final ObjectMapper JACKSON = getDefaultObjectMapper();
+  @Nullable private String systemPrompt = null;
 
   @Nonnull private final Destination destination;
 
@@ -154,11 +142,29 @@ public OpenAiChatCompletionOutput chatCompletion(
   }
 
   /**
-   * Generate a completion for the given prompt.
+   * Stream a completion for the given prompt. Returns a <b>lazily</b> populated stream of text
+   * chunks. To access more details about the individual chunks, use {@link
+   * #streamChatCompletionDeltas(OpenAiChatCompletionParameters)}.
+   *
+   * <p>The stream should be consumed using a try-with-resources block to ensure that the underlying
+   * HTTP connection is closed.
+   *
+   * <p>Example:
+   *
+   * <pre>{@code
+   * try (var stream = client.streamChatCompletion("...")) {
+   *       stream.forEach(System.out::println);
+   * }
+   * }</pre>
+   *
+   * <p>Please keep in mind that using a terminal stream operation like {@link Stream#forEach} will
+   * block until all chunks are consumed. Also, for obvious reasons, invoking {@link
+   * Stream#parallel()} on this stream is not supported.
    *
    * @param prompt a text message.
    * @return A stream of message deltas
    * @throws OpenAiClientException if the request fails or if the finish reason is content_filter
+   * @see #streamChatCompletionDeltas(OpenAiChatCompletionParameters)
    */
   @Nonnull
   public Stream<String> streamChatCompletion(@Nonnull final String prompt)
@@ -181,11 +187,31 @@ private static void throwOnContentFilter(@Nonnull final OpenAiChatCompletionDelt
   }
 
   /**
-   * Generate a completion for the given prompt.
+   * Stream a completion for the given prompt. Returns a <b>lazily</b> populated stream of delta
+   * objects. To simply stream the text chunks use {@link #streamChatCompletion(String)}
    *
-   * @param parameters the prompt, including messages and other parameters.
-   * @return A stream of chat completion delta elements.
-   * @throws OpenAiClientException if the request fails
+   * <p>The stream should be consumed using a try-with-resources block to ensure that the underlying
+   * HTTP connection is closed.
+   *
+   * <p>Example:
+   *
+   * <pre>{@code
+   * try (var stream = client.streamChatCompletionDeltas(params)) {
+   *       stream
+   *           .peek(delta -> System.out.println(delta.getUsage()))
+   *           .map(OpenAiChatCompletionDelta::getDeltaContent)
+   *           .forEach(System.out::println);
+   * }
+   * }</pre>
+   *
+   * <p>Please keep in mind that using a terminal stream operation like {@link Stream#forEach} will
+   * block until all chunks are consumed. Also, for obvious reasons, invoking {@link
+   * Stream#parallel()} on this stream is not supported.
+   *
+   * @param parameters The prompt, including messages and other parameters.
+   * @return A stream of message deltas
+   * @throws OpenAiClientException if the request fails or if the finish reason is content_filter
+   * @see #streamChatCompletion(String)
    */
   @Nonnull
   public Stream<OpenAiChatCompletionDelta> streamChatCompletionDeltas(

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientException.java

@@ -1,28 +1,7 @@
 package com.sap.ai.sdk.foundationmodels.openai;
 
-import java.io.Serial;
-import javax.annotation.Nonnull;
+import lombok.experimental.StandardException;
 
 /** Generic exception for errors occurring when using OpenAI foundation models. */
-public class OpenAiClientException extends RuntimeException {
-  @Serial private static final long serialVersionUID = -7345541120979974432L;
-
-  /**
-   * Create a new exception with the given message.
-   *
-   * @param message the message
-   */
-  public OpenAiClientException(@Nonnull final String message) {
-    super(message);
-  }
-
-  /**
-   * Create a new exception with the given message and cause.
-   *
-   * @param message the message
-   * @param e the cause
-   */
-  public OpenAiClientException(@Nonnull final String message, @Nonnull final Exception e) {
-    super(message, e);
-  }
-}
+@StandardException
+public class OpenAiClientException extends RuntimeException {}

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/model/DeltaAggregatable.java

@@ -1,5 +1,6 @@
 package com.sap.ai.sdk.foundationmodels.openai.model;
 
+import com.google.common.annotations.Beta;
 import javax.annotation.Nonnull;
 
 /**
@@ -9,6 +10,7 @@
  *
  * @param <D> the delta type.
  */
+@Beta
 public interface DeltaAggregatable<D> {
 
   /**

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/model/OpenAiChatCompletionOutput.java

@@ -47,11 +47,7 @@ public String getContent() throws OpenAiClientException {
     return Objects.requireNonNullElse(getChoices().get(0).getMessage().getContent(), "");
   }
 
-  /**
-   * Add a streamed delta to the total output.
-   *
-   * @param delta the delta to add.
-   */
+  @Override
   public void addDelta(@Nonnull final OpenAiChatCompletionDelta delta) {
     super.addDelta(delta);
 

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/model/OpenAiChatCompletionParameters.java

@@ -84,18 +84,6 @@ public class OpenAiChatCompletionParameters extends OpenAiCompletionParameters {
   @Nullable
   private ToolChoice toolChoice;
 
-  /**
-   * <b>NOTE:</b> This method is currently not supported. Therefore, it stays protected.<br>
-   * <br>
-   * The configuration entries for Azure OpenAI chat extensions that use them. This additional
-   * specification is only compatible with Azure OpenAI.
-   */
-  @JsonProperty("data_sources")
-  @Setter(onParam_ = @Nullable, value = AccessLevel.PROTECTED)
-  private List<Object> dataSources; // TODO for implementation details, please find
-
-  // https://github.com/Azure/azure-rest-api-specs/blob/3cb1b51638616435470fc10ea00de92512186ece/specification/cognitiveservices/data-plane/AzureOpenAI/inference/stable/2024-02-01/inference.json#L1223
-
   /** "response_format": { "type": "json_object" } */
   @JsonFormat(shape = JsonFormat.Shape.OBJECT)
   @RequiredArgsConstructor

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/model/OpenAiChatCompletionTool.java

@@ -25,7 +25,6 @@ public class OpenAiChatCompletionTool {
   private OpenAiChatCompletionFunction function;
 
   /** The type of the tool. Currently, only `function` is supported. */
-  // JSON payload sent: { "type": "function" }
   @RequiredArgsConstructor
   public enum ToolType {
     /**