Skip to content

Commit 56e9e2a

Browse files
committed
fix the problem of missing fields in ModelOptionsUtils.merge()
Signed-off-by: stillmoon <stillmoon516@foxmail.com>
1 parent d5a3269 commit 56e9e2a

File tree

2 files changed

+84
-11
lines changed

2 files changed

+84
-11
lines changed

spring-ai-model/src/main/java/org/springframework/ai/model/ModelOptionsUtils.java

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717
package org.springframework.ai.model;
1818

19+
import com.fasterxml.jackson.annotation.JsonAnyGetter;
1920
import java.beans.PropertyDescriptor;
2021
import java.lang.reflect.Field;
22+
import java.lang.reflect.Method;
2123
import java.lang.reflect.Type;
2224
import java.util.ArrayList;
2325
import java.util.Arrays;
@@ -83,7 +85,7 @@ public abstract class ModelOptionsUtils {
8385

8486
private static final List<String> BEAN_MERGE_FIELD_EXCISIONS = List.of("class");
8587

86-
private static final ConcurrentHashMap<Class<?>, List<String>> REQUEST_FIELD_NAMES_PER_CLASS = new ConcurrentHashMap<>();
88+
private static final ConcurrentHashMap<Class<?>, JsonPropertyResult> REQUEST_FIELD_NAMES_PER_CLASS = new ConcurrentHashMap<>();
8789

8890
private static final AtomicReference<SchemaGenerator> SCHEMA_GENERATOR_CACHE = new AtomicReference<>();
8991

@@ -182,10 +184,14 @@ public static <T> T merge(Object source, Object target, Class<T> clazz, List<Str
182184
}
183185

184186
List<String> requestFieldNames = CollectionUtils.isEmpty(acceptedFieldNames)
185-
? REQUEST_FIELD_NAMES_PER_CLASS.computeIfAbsent(clazz, ModelOptionsUtils::getJsonPropertyValues)
187+
? REQUEST_FIELD_NAMES_PER_CLASS.computeIfAbsent(clazz, ModelOptionsUtils::getJsonPropertyResult)
188+
.properties()
186189
: acceptedFieldNames;
187190

188-
if (CollectionUtils.isEmpty(requestFieldNames)) {
191+
boolean acceptAllFields = REQUEST_FIELD_NAMES_PER_CLASS.containsKey(clazz)
192+
&& REQUEST_FIELD_NAMES_PER_CLASS.get(clazz).acceptAllFields;
193+
194+
if (!acceptAllFields && CollectionUtils.isEmpty(requestFieldNames)) {
189195
throw new IllegalArgumentException("No @JsonProperty fields found in the " + clazz.getName());
190196
}
191197

@@ -199,7 +205,7 @@ public static <T> T merge(Object source, Object target, Class<T> clazz, List<Str
199205

200206
targetMap = targetMap.entrySet()
201207
.stream()
202-
.filter(e -> requestFieldNames.contains(e.getKey()))
208+
.filter(e -> acceptAllFields || requestFieldNames.contains(e.getKey()))
203209
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
204210

205211
return ModelOptionsUtils.mapToClass(targetMap, clazz);
@@ -263,20 +269,33 @@ public static <T> T mapToClass(Map<String, Object> source, Class<T> clazz) {
263269
}
264270

265271
/**
266-
* Returns the list of name values of the {@link JsonProperty} annotations.
272+
* Returns the result contains tag of accept all fields or list of json fields.
267273
* @param clazz the class that contains fields annotated with {@link JsonProperty}.
268-
* @return the list of values of the {@link JsonProperty} annotations.
274+
* @return the result contains tag of method has {@link JsonAnyGetter} annotation or
275+
* list of values of the {@link JsonProperty} annotations.
269276
*/
270-
public static List<String> getJsonPropertyValues(Class<?> clazz) {
277+
public static JsonPropertyResult getJsonPropertyResult(Class<?> clazz) {
271278
List<String> values = new ArrayList<>();
272279
Field[] fields = clazz.getDeclaredFields();
280+
Method[] methods = clazz.getDeclaredMethods();
281+
boolean hasAnyGetter = false;
282+
283+
for (Method method : methods) {
284+
// Iterate through the method to check JsonAnyGetter annotation to ensure that unknown parameters can be copied
285+
JsonAnyGetter anyGetterAnnotation = method.getAnnotation(JsonAnyGetter.class);
286+
if (anyGetterAnnotation != null) {
287+
hasAnyGetter = true;
288+
return new JsonPropertyResult(hasAnyGetter, values);
289+
}
290+
}
291+
273292
for (Field field : fields) {
274293
JsonProperty jsonPropertyAnnotation = field.getAnnotation(JsonProperty.class);
275294
if (jsonPropertyAnnotation != null) {
276295
values.add(jsonPropertyAnnotation.value());
277296
}
278297
}
279-
return values;
298+
return new JsonPropertyResult(false, values);
280299
}
281300

282301
/**
@@ -452,4 +471,10 @@ public static <T> T mergeOption(T runtimeValue, T defaultValue) {
452471
return ObjectUtils.isEmpty(runtimeValue) ? defaultValue : runtimeValue;
453472
}
454473

474+
/**
475+
* Record the decision result of {@link #getJsonPropertyResult(Class)}
476+
* @param acceptAllFields the current class allows tags for all properties
477+
* @param properties list of properties allowed by the current class
478+
*/
479+
public record JsonPropertyResult(boolean acceptAllFields, List<String> properties) {}
455480
}

spring-ai-model/src/test/java/org/springframework/ai/model/ModelOptionsUtilsTests.java

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
package org.springframework.ai.model;
1818

19+
import com.fasterxml.jackson.annotation.JsonAnyGetter;
20+
import com.fasterxml.jackson.annotation.JsonAnySetter;
21+
import java.util.HashMap;
1922
import java.util.Map;
2023

2124
import com.fasterxml.jackson.annotation.JsonProperty;
@@ -26,6 +29,7 @@
2629
import com.fasterxml.jackson.databind.json.JsonMapper;
2730
import org.junit.jupiter.api.Test;
2831

32+
import static org.assertj.core.api.Assertions.as;
2933
import static org.assertj.core.api.Assertions.assertThat;
3034
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3135

@@ -45,6 +49,9 @@ public void merge() {
4549
specificOptions.setName("Mike");
4650
specificOptions.setSpecificField("SpecificField");
4751

52+
TestSpecificOptions extraOptions = new TestSpecificOptions();
53+
extraOptions.setExtraBodyProperty("key", "value");
54+
4855
assertThatThrownBy(
4956
() -> ModelOptionsUtils.merge(portableOptions, specificOptions, TestPortableOptionsImpl.class))
5057
.isInstanceOf(IllegalArgumentException.class)
@@ -56,6 +63,10 @@ public void merge() {
5663
assertThat(specificOptions2.getName()).isEqualTo("John"); // !!! Overridden by the
5764
// portableOptions
5865
assertThat(specificOptions2.getSpecificField()).isEqualTo("SpecificField");
66+
67+
var extraSpecOptions = ModelOptionsUtils.merge(extraOptions, specificOptions, TestSpecificOptions.class);
68+
assertThat(extraSpecOptions.extraBody()).containsKey("key");
69+
assertThat(extraSpecOptions.extraBody().get("key")).isEqualTo("value");
5970
}
6071

6172
@Test
@@ -171,12 +182,22 @@ public void pojo_emptyStringAsNullObject() throws Exception {
171182
}
172183

173184
@Test
174-
public void getJsonPropertyValues() {
185+
public void getJsonPropertyResult() {
175186
record TestRecord(@JsonProperty("field1") String fieldA, @JsonProperty("field2") String fieldB) {
176187

177188
}
178-
assertThat(ModelOptionsUtils.getJsonPropertyValues(TestRecord.class)).hasSize(2);
179-
assertThat(ModelOptionsUtils.getJsonPropertyValues(TestRecord.class)).containsExactly("field1", "field2");
189+
assertThat(ModelOptionsUtils.getJsonPropertyResult(TestRecord.class).properties()).hasSize(2);
190+
assertThat(ModelOptionsUtils.getJsonPropertyResult(TestRecord.class).properties()).containsExactly("field1", "field2");
191+
192+
record TestAnyGetterRecord(Map<String, Object> extraBody) {
193+
194+
@JsonAnyGetter
195+
public Map<String, Object> extraBody() {
196+
return this.extraBody;
197+
}
198+
}
199+
200+
assertThat(ModelOptionsUtils.getJsonPropertyResult(TestAnyGetterRecord.class).acceptAllFields()).isEqualTo(true);
180201
}
181202

182203
@Test
@@ -357,6 +378,8 @@ public static class TestSpecificOptions implements TestPortableOptions {
357378
@JsonProperty("age")
358379
private Integer age;
359380

381+
private Map<String, Object> extraBody = new HashMap<>();
382+
360383
@Override
361384
public String getName() {
362385
return this.name;
@@ -385,6 +408,31 @@ public void setSpecificField(String modelSpecificField) {
385408
this.specificField = modelSpecificField;
386409
}
387410

411+
/**
412+
* Overrides the default accessor to add @JsonAnyGetter annotation.
413+
* This causes Jackson to flatten the extraBody map contents to the top level of the JSON,
414+
* matching the behavior expected by OpenAI-compatible servers like vLLM, Ollama, etc.
415+
* @return The extraBody map, or null if not set.
416+
*/
417+
@JsonAnyGetter
418+
public Map<String, Object> extraBody() {
419+
return this.extraBody;
420+
}
421+
422+
/**
423+
* Handles deserialization of unknown properties into the extraBody map.
424+
* This enables JSON with extra fields to be deserialized into ChatCompletionRequest,
425+
* which is useful for implementing OpenAI API proxy servers with @RestController.
426+
* @param key The property name
427+
* @param value The property value
428+
*/
429+
@JsonAnySetter
430+
private void setExtraBodyProperty(String key, Object value) {
431+
if (this.extraBody != null) {
432+
this.extraBody.put(key, value);
433+
}
434+
}
435+
388436
@Override
389437
public String toString() {
390438
return "TestModelSpecificOptions{" + "specificField='" + this.specificField + '\'' + ", name='" + this.name

0 commit comments

Comments
 (0)