Affects: \every version up to 3.3.0-SNAPSHOT (tested with both 3.2.3 and 3.3.0-SNAPSHOT)
Hello!
I came across an issue with nested records used as query parameters. Here is what my Controller looks like:
@RestController
@RequestMapping("/api")
public class DemoController {
@GetMapping
public void test(RecordParamHolder paramHolder) {
System.out.println("Record: " + paramHolder.recordParamName()); // not working when passing indexed param list
System.out.println("POJO: " + paramHolder.pojoParamName()); // always working
}
}
RecordParamHolder looks like this (as the names state 1st one is a POJO and the 2nd one is a record):
public record RecordParamHolder(
PojoListParam pojoParamName,
RecordListParam recordParamName
) {}
Both PojoListParam and RecordListParam contain one field of type List<String>. And so when I try to pass a list of params like this: GET localhost:8080/api?recordParamName.in%5B0%5D=abc (square brackets escaped) I see that the list is not being resolved and is NULL. However, similar request works fine for nested POJO class – GET localhost:8080/api?pojoParamName.in%5B0%5D=abc.
I know that I can pass a list like this: paramName=value1,value2,etc., but this approach doesn't work when there is a comma in any of the values (see this issue for example #29411). I know I can also pass it like this paramName=value1¶mName=value2&etc, but I was told it's inconvenient to build such request from JavaScript because paramName is being overwritten and only the last one is passed.
Anyway, I believe it should work properly for nested records just as it works for nested POJOs. Attaching a Minimal Reproducible Example.
Thanks in advance!
Comment From: quaff
Actually It failed if param name contains index (auto-growing) and no corresponding setter method, no matter it's nested or not, no matter it's record or not.
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.junit.jupiter.api.Test;
import org.springframework.beans.MutablePropertyValues;
import org.springframework.core.ResolvableType;
import org.springframework.validation.DataBinder;
import org.springframework.web.bind.WebDataBinder;
import static org.assertj.core.api.Assertions.assertThat;
class WebDataBindTests {
@Test
void nested() {
Map<String, String> data = new HashMap<>();
data.put("pojo.list[0]", "test");
data.put("record.list[0]", "test");
data.put("pojoWithoutSetter.list[0]", "test");
WebDataBinder binder = new WebDataBinder(null, "test");
binder.setTargetType(ResolvableType.forClass(Holder.class));
binder.construct(new DataBinder.ValueResolver() {
@Override
public Object resolveValue(String name, Class<?> type) {
return data.get(name);
}
@Override
public Set<String> getNames() {
return data.keySet();
}
});
binder.bind(new MutablePropertyValues(data));
Holder holder = (Holder) binder.getTarget();
assertThat(holder.pojo.getList()).first().isEqualTo("test"); // works
assertThat(holder.record.list()).first().isEqualTo("test"); // failed
assertThat(holder.pojoWithoutSetter.getList()).first().isEqualTo("test"); // failed
}
@Test
void topLevelRecord() {
Map<String, String> data = new HashMap<>();
data.put("list[0]", "test");
WebDataBinder binder = new WebDataBinder(null, "test");
binder.setTargetType(ResolvableType.forClass(Record.class));
binder.construct(new DataBinder.ValueResolver() {
@Override
public Object resolveValue(String name, Class<?> type) {
return data.get(name);
}
@Override
public Set<String> getNames() {
return data.keySet();
}
});
binder.bind(new MutablePropertyValues(data));
Record record = (Record) binder.getTarget();
assertThat(record.list()).first().isEqualTo("test");
}
@Test
void topLevelPojoWithoutSetter() {
Map<String, String> data = new HashMap<>();
data.put("list[0]", "test");
WebDataBinder binder = new WebDataBinder(null, "test");
binder.setTargetType(ResolvableType.forClass(PojoWithoutSetter.class));
binder.construct(new DataBinder.ValueResolver() {
@Override
public Object resolveValue(String name, Class<?> type) {
return data.get(name);
}
@Override
public Set<String> getNames() {
return data.keySet();
}
});
binder.bind(new MutablePropertyValues(data));
PojoWithoutSetter pojoWithoutSetter = (PojoWithoutSetter) binder.getTarget();
assertThat(pojoWithoutSetter.getList()).first().isEqualTo("test");
}
record Holder(Pojo pojo, Record record, PojoWithoutSetter pojoWithoutSetter) {
}
record Record(List<String> list) {
}
static class Pojo {
private List<String> list;
public List<String> getList() {
return this.list;
}
public void setList(List<String> list) {
this.list = list;
}
}
static class PojoWithoutSetter {
private final List<String> list;
PojoWithoutSetter(List<String> list) {
this.list = list;
}
public List<String> getList() {
return this.list;
}
}
}
Comment From: anthsup
@quaff Thanks for the info! So are you saying it can't possibly work with records because of their immutability?
Comment From: snicoll
The POJO is mutable so every parameter that is handled mutate the existing instance. Your record needs to be build with the list of things so you can't by desing provide indexed values. You would be able to call an API on your record to do the same thing if you did this manually.
Comment From: quaff
I think it's possible to support that since list is present with ValueResolver::getNames, then DataBinder should create an empty ArrayList instead of null passing into the constructor, and everything works as expected now, I've created PR https://github.com/spring-projects/spring-framework/pull/32386 to fix it. WDYT @rstoyanchev
Here is the workaround, @anthsup You can check null in record constructor.
record Record(List<String> list) {
public Record {
if (list == null) {
list = new ArrayList<>();
}
}
}
static class PojoWithoutSetter {
private final List<String> list;
PojoWithoutSetter(List<String> list) {
if (list == null)
list = new ArrayList<>();
this.list = list;
}
public List<String> getList() {
return this.list;
}
}
Comment From: sbrannen
- superseded by #32386
Comment From: rstoyanchev
Actually superseded by #32426.