Skip to content

Commit 96af7f3

Browse files
sdeleuzesdeleuze
committed
Fix OpenAI generic options merging
Ensure proper handling of generic merging in audio, image, embedding and moderation options. Closes #6042 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
1 parent 2d38075 commit 96af7f3

9 files changed

Lines changed: 206 additions & 46 deletions

File tree

‎models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiAudioSpeechOptions.java‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,19 @@ public Builder merge(@Nullable TextToSpeechOptions from) {
247247
if (from == null) {
248248
return this;
249249
}
250-
if (from instanceof OpenAiAudioSpeechOptions castFrom) {
251-
// Parent class fields
250+
if (from.getModel() != null) {
251+
this.model = from.getModel();
252+
}
253+
if (from.getVoice() != null) {
254+
this.voice = from.getVoice();
255+
}
256+
if (from.getFormat() != null) {
257+
this.responseFormat = from.getFormat();
258+
}
259+
if (from.getSpeed() != null) {
260+
this.speed = from.getSpeed();
261+
}
262+
if (from instanceof AbstractOpenAiOptions castFrom) {
252263
if (castFrom.getBaseUrl() != null) {
253264
this.baseUrl = castFrom.getBaseUrl();
254265
}
@@ -258,9 +269,6 @@ public Builder merge(@Nullable TextToSpeechOptions from) {
258269
if (castFrom.getCredential() != null) {
259270
this.credential = castFrom.getCredential();
260271
}
261-
if (castFrom.getModel() != null) {
262-
this.model = castFrom.getModel();
263-
}
264272
if (castFrom.getDeploymentName() != null) {
265273
this.microsoftDeploymentName = castFrom.getDeploymentName();
266274
}
@@ -282,19 +290,14 @@ public Builder merge(@Nullable TextToSpeechOptions from) {
282290
if (castFrom.getCustomHeaders() != null) {
283291
this.customHeaders = castFrom.getCustomHeaders();
284292
}
285-
// Child class fields
293+
}
294+
if (from instanceof OpenAiAudioSpeechOptions castFrom) {
286295
if (castFrom.getInput() != null) {
287296
this.input = castFrom.getInput();
288297
}
289-
if (castFrom.getVoice() != null) {
290-
this.voice = castFrom.getVoice();
291-
}
292298
if (castFrom.getResponseFormat() != null) {
293299
this.responseFormat = castFrom.getResponseFormat();
294300
}
295-
if (castFrom.getSpeed() != null) {
296-
this.speed = castFrom.getSpeed();
297-
}
298301
}
299302
return this;
300303
}

‎models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiAudioTranscriptionOptions.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ public Builder merge(@Nullable AudioTranscriptionOptions from) {
205205
if (from.getModel() != null) {
206206
this.model = from.getModel();
207207
}
208-
if (from instanceof OpenAiAudioTranscriptionOptions castFrom) {
208+
if (from instanceof AbstractOpenAiOptions castFrom) {
209209
if (castFrom.getBaseUrl() != null) {
210210
this.baseUrl = castFrom.getBaseUrl();
211211
}
@@ -236,6 +236,8 @@ public Builder merge(@Nullable AudioTranscriptionOptions from) {
236236
if (castFrom.getCustomHeaders() != null) {
237237
this.customHeaders = castFrom.getCustomHeaders();
238238
}
239+
}
240+
if (from instanceof OpenAiAudioTranscriptionOptions castFrom) {
239241
if (castFrom.getResponseFormat() != null) {
240242
this.responseFormat = castFrom.getResponseFormat();
241243
}

‎models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiEmbeddingOptions.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,13 @@ public Builder merge(@Nullable EmbeddingOptions from) {
177177
if (from == null) {
178178
return this;
179179
}
180-
if (from instanceof OpenAiEmbeddingOptions castFrom) {
181-
// Parent class fields
180+
if (from.getModel() != null) {
181+
this.model = from.getModel();
182+
}
183+
if (from.getDimensions() != null) {
184+
this.dimensions = from.getDimensions();
185+
}
186+
if (from instanceof AbstractOpenAiOptions castFrom) {
182187
if (castFrom.getBaseUrl() != null) {
183188
this.baseUrl = castFrom.getBaseUrl();
184189
}
@@ -188,9 +193,6 @@ public Builder merge(@Nullable EmbeddingOptions from) {
188193
if (castFrom.getCredential() != null) {
189194
this.credential = castFrom.getCredential();
190195
}
191-
if (castFrom.getModel() != null) {
192-
this.model = castFrom.getModel();
193-
}
194196
if (castFrom.getDeploymentName() != null) {
195197
this.microsoftDeploymentName = castFrom.getDeploymentName();
196198
}
@@ -212,16 +214,14 @@ public Builder merge(@Nullable EmbeddingOptions from) {
212214
if (castFrom.getCustomHeaders() != null) {
213215
this.customHeaders = castFrom.getCustomHeaders();
214216
}
215-
// Child class fields
217+
}
218+
if (from instanceof OpenAiEmbeddingOptions castFrom) {
216219
if (castFrom.getUser() != null) {
217220
this.user = castFrom.getUser();
218221
}
219222
if (castFrom.getEncodingFormat() != null) {
220223
this.encodingFormat = castFrom.getEncodingFormat();
221224
}
222-
if (castFrom.getDimensions() != null) {
223-
this.dimensions = castFrom.getDimensions();
224-
}
225225
}
226226
return this;
227227
}

‎models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiImageOptions.java‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,25 @@ public Builder merge(@Nullable ImageOptions from) {
269269
if (from == null) {
270270
return this;
271271
}
272-
if (from instanceof OpenAiImageOptions castFrom) {
273-
// Parent class fields
272+
if (from.getModel() != null) {
273+
this.model = from.getModel();
274+
}
275+
if (from.getN() != null) {
276+
this.n = from.getN();
277+
}
278+
if (from.getWidth() != null) {
279+
this.width = from.getWidth();
280+
}
281+
if (from.getHeight() != null) {
282+
this.height = from.getHeight();
283+
}
284+
if (from.getResponseFormat() != null) {
285+
this.responseFormat = from.getResponseFormat();
286+
}
287+
if (from.getStyle() != null) {
288+
this.style = from.getStyle();
289+
}
290+
if (from instanceof AbstractOpenAiOptions castFrom) {
274291
if (castFrom.getBaseUrl() != null) {
275292
this.baseUrl = castFrom.getBaseUrl();
276293
}
@@ -280,9 +297,6 @@ public Builder merge(@Nullable ImageOptions from) {
280297
if (castFrom.getCredential() != null) {
281298
this.credential = castFrom.getCredential();
282299
}
283-
if (castFrom.getModel() != null) {
284-
this.model = castFrom.getModel();
285-
}
286300
if (castFrom.getDeploymentName() != null) {
287301
this.microsoftDeploymentName = castFrom.getDeploymentName();
288302
}
@@ -304,28 +318,14 @@ public Builder merge(@Nullable ImageOptions from) {
304318
if (castFrom.getCustomHeaders() != null) {
305319
this.customHeaders = castFrom.getCustomHeaders();
306320
}
307-
// Child class fields
308-
if (castFrom.getN() != null) {
309-
this.n = castFrom.getN();
310-
}
311-
if (castFrom.getWidth() != null) {
312-
this.width = castFrom.getWidth();
313-
}
314-
if (castFrom.getHeight() != null) {
315-
this.height = castFrom.getHeight();
316-
}
321+
}
322+
if (from instanceof OpenAiImageOptions castFrom) {
317323
if (castFrom.getQuality() != null) {
318324
this.quality = castFrom.getQuality();
319325
}
320-
if (castFrom.getResponseFormat() != null) {
321-
this.responseFormat = castFrom.getResponseFormat();
322-
}
323326
if (castFrom.getSize() != null) {
324327
this.size = castFrom.getSize();
325328
}
326-
if (castFrom.getStyle() != null) {
327-
this.style = castFrom.getStyle();
328-
}
329329
if (castFrom.getUser() != null) {
330330
this.user = castFrom.getUser();
331331
}

‎models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiModerationOptions.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ public Builder merge(@Nullable ModerationOptions options) {
124124
if (options.getModel() != null) {
125125
this.model = options.getModel();
126126
}
127-
if (options instanceof OpenAiModerationOptions castFrom) {
127+
if (options instanceof AbstractOpenAiOptions castFrom) {
128+
128129
if (castFrom.getBaseUrl() != null) {
129130
this.baseUrl = castFrom.getBaseUrl();
130131
}
@@ -134,8 +135,8 @@ public Builder merge(@Nullable ModerationOptions options) {
134135
if (castFrom.getCredential() != null) {
135136
this.credential = castFrom.getCredential();
136137
}
137-
if (castFrom.getMicrosoftDeploymentName() != null) {
138-
this.microsoftDeploymentName = castFrom.getMicrosoftDeploymentName();
138+
if (castFrom.getDeploymentName() != null) {
139+
this.microsoftDeploymentName = castFrom.getDeploymentName();
139140
}
140141
if (castFrom.getMicrosoftFoundryServiceVersion() != null) {
141142
this.microsoftFoundryServiceVersion = castFrom.getMicrosoftFoundryServiceVersion();
@@ -156,6 +157,9 @@ public Builder merge(@Nullable ModerationOptions options) {
156157
this.customHeaders = castFrom.getCustomHeaders();
157158
}
158159
}
160+
if (options instanceof OpenAiModerationOptions castFrom) {
161+
// No specific properties to merge for now
162+
}
159163
return this;
160164
}
161165

‎models/spring-ai-openai/src/test/java/org/springframework/ai/openai/audio/speech/OpenAiAudioSpeechOptionsTests.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Copyright 2023-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.ai.openai.audio.speech;
18+
19+
import org.junit.jupiter.api.Test;
20+
21+
import org.springframework.ai.audio.tts.TextToSpeechOptions;
22+
import org.springframework.ai.openai.OpenAiAudioSpeechOptions;
23+
24+
import static org.assertj.core.api.Assertions.assertThat;
25+
26+
class OpenAiAudioSpeechOptionsTests {
27+
28+
@Test
29+
void genericAudioSpeechOptionsAreMerged() {
30+
TextToSpeechOptions source = TextToSpeechOptions.builder()
31+
.model("generic-model")
32+
.voice("generic-voice")
33+
.format("mp3")
34+
.speed(1.5)
35+
.build();
36+
37+
OpenAiAudioSpeechOptions merged = OpenAiAudioSpeechOptions.builder().merge(source).build();
38+
39+
assertThat(merged.getModel()).isEqualTo("generic-model");
40+
assertThat(merged.getVoice()).isEqualTo("generic-voice");
41+
assertThat(merged.getResponseFormat()).isEqualTo("mp3");
42+
assertThat(merged.getSpeed()).isEqualTo(1.5);
43+
}
44+
45+
}

‎models/spring-ai-openai/src/test/java/org/springframework/ai/openai/audio/transcription/OpenAiAudioTranscriptionOptionsTests.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2023-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.ai.openai.audio.transcription;
18+
19+
import org.junit.jupiter.api.Test;
20+
21+
import org.springframework.ai.audio.transcription.AudioTranscriptionOptions;
22+
import org.springframework.ai.openai.OpenAiAudioTranscriptionOptions;
23+
24+
import static org.assertj.core.api.Assertions.assertThat;
25+
26+
class OpenAiAudioTranscriptionOptionsTests {
27+
28+
@Test
29+
void genericAudioTranscriptionOptionsAreMerged() {
30+
AudioTranscriptionOptions source = new AudioTranscriptionOptions() {
31+
@Override
32+
public String getModel() {
33+
return "generic-model";
34+
}
35+
};
36+
37+
OpenAiAudioTranscriptionOptions merged = OpenAiAudioTranscriptionOptions.builder().merge(source).build();
38+
39+
assertThat(merged.getModel()).isEqualTo("generic-model");
40+
}
41+
42+
}

‎models/spring-ai-openai/src/test/java/org/springframework/ai/openai/embedding/OpenAiEmbeddingOptionsTests.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,18 @@ void encodingFormatIsCopiedAndMerged() {
6363
assertThat(merged.getEncodingFormat()).isEqualTo(OpenAiEmbeddingOptions.EncodingFormat.FLOAT);
6464
}
6565

66+
@Test
67+
void genericEmbeddingOptionsAreMerged() {
68+
org.springframework.ai.embedding.EmbeddingOptions source = org.springframework.ai.embedding.EmbeddingOptions
69+
.builder()
70+
.model("generic-model")
71+
.dimensions(1024)
72+
.build();
73+
74+
OpenAiEmbeddingOptions merged = OpenAiEmbeddingOptions.builder().merge(source).build();
75+
76+
assertThat(merged.getModel()).isEqualTo("generic-model");
77+
assertThat(merged.getDimensions()).isEqualTo(1024);
78+
}
79+
6680
}

‎models/spring-ai-openai/src/test/java/org/springframework/ai/openai/image/OpenAiImageOptionsTests.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright 2023-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.ai.openai.image;
18+
19+
import org.junit.jupiter.api.Test;
20+
21+
import org.springframework.ai.image.ImageOptions;
22+
import org.springframework.ai.image.ImageOptionsBuilder;
23+
import org.springframework.ai.openai.OpenAiImageOptions;
24+
25+
import static org.assertj.core.api.Assertions.assertThat;
26+
27+
class OpenAiImageOptionsTests {
28+
29+
@Test
30+
void genericImageOptionsAreMerged() {
31+
ImageOptions source = ImageOptionsBuilder.builder()
32+
.model("generic-model")
33+
.N(2)
34+
.width(1024)
35+
.height(1024)
36+
.responseFormat("b64_json")
37+
.style("vivid")
38+
.build();
39+
40+
OpenAiImageOptions merged = OpenAiImageOptions.builder().merge(source).build();
41+
42+
assertThat(merged.getModel()).isEqualTo("generic-model");
43+
assertThat(merged.getN()).isEqualTo(2);
44+
assertThat(merged.getWidth()).isEqualTo(1024);
45+
assertThat(merged.getHeight()).isEqualTo(1024);
46+
assertThat(merged.getResponseFormat()).isEqualTo("b64_json");
47+
assertThat(merged.getStyle()).isEqualTo("vivid");
48+
}
49+
50+
}

0 commit comments

Comments
 (0)