From 95eb9fd6a350ef4e336b301ea0fd2d148e63dded Mon Sep 17 00:00:00 2001 From: Dennys Fredericci Date: Thu, 28 Nov 2024 08:44:33 +0100 Subject: [PATCH] Fix OpenAI image dimension handling The OpenAiImageOptions class previously had inconsistent behavior when setting width and height properties. This change ensures that the size property is only computed when both dimensions are available, preventing invalid states in the API requests. Key changes: - Update setWidth/setHeight to only set size when both are non-null - Add test cases to verify null size when dimensions are incomplete - Improve code organization with proper import conventions This change aligns with OpenAI's API requirements for image generation where both dimensions must be specified for valid requests. --- .../ai/openai/OpenAiImageOptions.java | 39 +++++++++++---- .../ai/openai/OpenAiImageOptionsTests.java | 48 ++++++++++++++++--- 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiImageOptions.java b/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiImageOptions.java index 774dc5baf..b587a4fd0 100644 --- a/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiImageOptions.java +++ b/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiImageOptions.java @@ -16,13 +16,12 @@ package org.springframework.ai.openai; -import java.util.Objects; - import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; - import org.springframework.ai.image.ImageOptions; +import java.util.Objects; + /** * OpenAI Image API options. OpenAiImageOptions.java * @@ -48,12 +47,18 @@ public class OpenAiImageOptions implements ImageOptions { /** * The width of the generated images. Must be one of 256, 512, or 1024 for dall-e-2. + * This property is interconnected with the 'size' property - setting both width and height + * will automatically compute and set the size in "widthxheight" format. Conversely, + * setting a valid size string will parse and set the individual width and height values. */ @JsonProperty("size_width") private Integer width; /** * The height of the generated images. Must be one of 256, 512, or 1024 for dall-e-2. + * This property is interconnected with the 'size' property - setting both width and height + * will automatically compute and set the size in "widthxheight" format. Conversely, + * setting a valid size string will parse and set the individual width and height values. */ @JsonProperty("size_height") private Integer height; @@ -76,6 +81,10 @@ public class OpenAiImageOptions implements ImageOptions { /** * The size of the generated images. Must be one of 256x256, 512x512, or 1024x1024 for * dall-e-2. Must be one of 1024x1024, 1792x1024, or 1024x1792 for dall-e-3 models. + * This property is automatically computed when both width and height are set, following + * the format "widthxheight". When setting this property directly, it must follow the + * format "WxH" where W and H are valid integers. Invalid formats will result in null + * width and height values. */ @JsonProperty("size") private String size; @@ -142,9 +151,13 @@ public class OpenAiImageOptions implements ImageOptions { } else if (this.size != null) { try { - return Integer.parseInt(this.size.split("x")[0]); + String[] dimensions = this.size.split("x"); + if (dimensions.length != 2) { + return null; + } + return Integer.parseInt(dimensions[0]); } - catch (NumberFormatException ex) { + catch (Exception ex) { return null; } } @@ -153,7 +166,9 @@ public class OpenAiImageOptions implements ImageOptions { public void setWidth(Integer width) { this.width = width; - this.size = this.width + "x" + this.height; + if (this.width != null && this.height != null) { + this.size = this.width + "x" + this.height; + } } @Override @@ -163,9 +178,13 @@ public class OpenAiImageOptions implements ImageOptions { } else if (this.size != null) { try { - return Integer.parseInt(this.size.split("x")[1]); + String[] dimensions = this.size.split("x"); + if (dimensions.length != 2) { + return null; + } + return Integer.parseInt(dimensions[1]); } - catch (NumberFormatException ex) { + catch (Exception ex) { return null; } } @@ -174,7 +193,9 @@ public class OpenAiImageOptions implements ImageOptions { public void setHeight(Integer height) { this.height = height; - this.size = this.width + "x" + this.height; + if (this.width != null && this.height != null) { + this.size = this.width + "x" + this.height; + } } @Override diff --git a/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/OpenAiImageOptionsTests.java b/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/OpenAiImageOptionsTests.java index f11613fa1..86ebbcc21 100644 --- a/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/OpenAiImageOptionsTests.java +++ b/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/OpenAiImageOptionsTests.java @@ -58,10 +58,10 @@ class OpenAiImageOptionsTests { void whenWidthIsSet() { OpenAiImageOptions options = new OpenAiImageOptions(); options.setWidth(1920); - assertThat(options.getHeight()).isEqualTo(null); + assertThat(options.getHeight()).isNull(); assertThat(options.getWidth()).isEqualTo(1920); - // This is because "setWidth()" computes "size" without checking for null values. - assertThat(options.getSize()).isEqualTo("1920xnull"); + // 1920xnull is not a valid size, so "size" should be null. + assertThat(options.getSize()).isNull(); } @Test @@ -69,9 +69,45 @@ class OpenAiImageOptionsTests { OpenAiImageOptions options = new OpenAiImageOptions(); options.setHeight(1080); assertThat(options.getHeight()).isEqualTo(1080); - assertThat(options.getWidth()).isEqualTo(null); - // This is because "setHeight()" computes "size" without checking for null values. - assertThat(options.getSize()).isEqualTo("nullx1080"); + assertThat(options.getWidth()).isNull(); + // nullx1080 is not a valid size, so "size" should be null. + assertThat(options.getSize()).isNull(); + } + + @Test + void whenInvalidSizeFormatIsSet() { + OpenAiImageOptions options = new OpenAiImageOptions(); + options.setSize("invalid"); + assertThat(options.getHeight()).isNull(); + assertThat(options.getWidth()).isNull(); + assertThat(options.getSize()).isEqualTo("invalid"); + } + + @Test + void whenSizeWithInvalidNumbersIsSet() { + OpenAiImageOptions options = new OpenAiImageOptions(); + options.setSize("axb"); + assertThat(options.getHeight()).isNull(); + assertThat(options.getWidth()).isNull(); + assertThat(options.getSize()).isEqualTo("axb"); + } + + @Test + void whenSizeWithMissingDimensionIsSet() { + OpenAiImageOptions options = new OpenAiImageOptions(); + options.setSize("1024x"); + assertThat(options.getHeight()).isNull(); + assertThat(options.getWidth()).isNull(); + assertThat(options.getSize()).isEqualTo("1024x"); + } + + @Test + void whenSizeWithExtraSeparatorsIsSet() { + OpenAiImageOptions options = new OpenAiImageOptions(); + options.setSize("1024x1024x1024"); + assertThat(options.getHeight()).isNull(); + assertThat(options.getWidth()).isNull(); + assertThat(options.getSize()).isEqualTo("1024x1024x1024"); } }