Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multidimensional arrays and maps #5

Open
poikilotherm opened this issue Apr 16, 2018 · 8 comments
Open

Support multidimensional arrays and maps #5

poikilotherm opened this issue Apr 16, 2018 · 8 comments

Comments

@poikilotherm
Copy link
Contributor

Dear @f-lopes,

the addition of support for Map types in #3 is great! In future forms, I will need to test with multidimensional arrays and maps (Array of Arrays, Map of Maps or even more dimensions).

Would you agree that adding support for them would respond to a common pattern of data usage? (At least I know this concept is heavily used within the PHP/Python/Perl world...)

Cheers,
Poikilotherm

@poikilotherm
Copy link
Contributor Author

@f-lopes would you welcome a PR for this? I actually need this... 😉

@f-lopes
Copy link
Owner

f-lopes commented Apr 17, 2018

Dear @poikilotherm,

Of course 😃, I will commit my changes soon (#4) to make sure you work on the latest codebase.

@poikilotherm
Copy link
Contributor Author

Hey @f-lopes

I started to work on this in https://github.com/poikilotherm/spring-mvc-test-utils/tree/support_mapOmap.

Unfortanetly, I can't get the Smoke Test fullTest() to work, because the three dimensional map seems not to be supported. While using a two dim map, this is no problem at all... 🤔

I tried to use such three dimensional maps in real forms and that was no problem at all... I just don't get it - maybe you have an idea?

@f-lopes
Copy link
Owner

f-lopes commented Apr 17, 2018

Hi @poikilotherm,

I saw your work on the fork. Multi-dimensional Map parameters need to be separated by dots "." to instantiate nested properties.
The smoke test is passing when adding the following parameter:
mmData[orgUnit].[employee].[firstName]=John

Could you first write unit tests to validate the three-dimensional map mapping to HTTP parameters?

Thanks.

@poikilotherm
Copy link
Contributor Author

poikilotherm commented Apr 19, 2018

Hi @f-lopes,

I already wrote that unit test first... ;-)

Actually I think we cannot use more than two dimensions. After testing this again with some example code, I can use maps with more than 2 dimensions within templates (SpEL seems to be ok with that when used in Thymeleaf), but when I try to submit the data, the binding fails with the exact same error.

I was not able to find any documentation on this and after digging through the Spring Framework source code, I was even more confused of the black magic that happens in the data binding background. Even in the Spring JIRA issues I was not able to find anything... Do you know some of the devs, so we might ask them directly? Not sure if they would react here on GitHub if we mention them...

Using the "dotted" notation you mentioned above might get things to work, but this is no good, as Thymeleaf etc are not using this format (they use the name[key1][key2][...] notation).

So maybe lets stick with 2 dimensions for now? (Will have to redesign my models, but well... things happen...)

@f-lopes
Copy link
Owner

f-lopes commented Apr 19, 2018

Hi @poikilotherm,

Sorry, I didn't find the unit test at first ;)

Of course, your solution may be to use a List to model your data. Is the problem related to the "auto-growing/instantiation" of nested Map properties. These discussions might help:

Unfortunately, I don't know any developer from the Spring team.

For your information, I will open a pull request to give you the possibility to discuss the next version (2.0.0) which will rely on Spring's TypeConverter implementation.

@poikilotherm
Copy link
Contributor Author

Thank you for the links @f-lopes. Unfortanetly I already had a look at those while digging through Google and they didn't enlighten me regarding this specific problem. The upstream bug report is closed as fixed, too...

As a consequence, I opened a new issue: https://jira.spring.io/browse/SPR-16745

Let's keep our fingers crossed some of the devs find the time to respond.

@f-lopes
Copy link
Owner

f-lopes commented Apr 20, 2018

Thanks for the issue.

I've created a pull-request for the next release (#7). Feel free to add your suggestions.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants