Nested record containing a query param of type List not being resolved correctly

[anthsup]

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&paramName=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!

[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;
		}

	}

}

[anthsup]

@quaff Thanks for the info! So are you saying it can't possibly work with records because of their immutability?

[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.

[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 #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;
		}

	}

[sbrannen]

• superseded by Support binding indexed values for record class #32386