Added converters.ObjectConverter and converters.ArrayConverter

[huxi]

Added "objects" sample application.
The "objects" sample shows usage of ObjectResolver, ObjectConverter and ArrayConverter.

Added ObjectConverterTest and ArrayConverterTest.

ArrayConverter uses reflection and throws an Error if it is used but Java 6 is not available.

[huxi]

Any chance of getting this merged and released in the foreseeable future?
We use the code above extensively in production and didn't stumble upon any problems. Is my pull request lacking in some way?

[huxi]

Created SHL-109 - Add converters.ObjectConverter and converters.ArrayConverter to complete aribitrary instances referencing this pull request.

[markpollack]

just getting around to it....

[ericbottard]

Closing, will address array/collection with https://jira.spring.io/browse/SHL-173 (which should be automatic)
ObjectConverter and coupling with DAO is too specific IMHO

[huxi]

There is no coupling with any DAO. ObjectResolver can wrap a DAO but doesn't have to. The FooRepository in the example has been used as an... example.

[huxi]

Thanks for breaking my code with the latest 1.2.0 release. 👍

[huxi]

Also thanks for zero documentation in either the Spring Shell Documentation or the javadoc of org.springframework.shell.converters.ArrayConverter.

Keep up the tremendous work. 👍

[markpollack]

Hi. This project is really at a minimal level of support on our side, that is the reality. I appreciate the PRs and know it is going way back to 2013. so yea, it is just basic neglect on our side, I accept responsibility. The sarcasm is certainly not appreciated as we are indeed trying our best here.

[ericbottard]

You don't have to upgrade to the latest version of Spring Shell. Moreover, org.springframework.shell.converters.ArrayConverter (in addition to being documented) provides support for the original usecase this PR was trying to address (albeit with too much complexity IMHO, which is why it was rejected).

[huxi]

Sorry about the sarcasm. I'm usually not like that, especially not while interacting with open source projects. But the way this was handled really annoyed me.

It's not so much that the reaction time was slow but about the way the PR was shot down.

My PR...

• adhered to the coding style
• contained tests
• contained its own sample app

My implementation supported two options in ArrayConverter, CASE_SENSITIVE (should the array value IDs be handled case-sensitive or not) and UNIQUE_VALUES (should the result array be allowed to contain duplicate values or not?). It's also returning properly typed T[] instead of Object[].

Supporting the CASE_SENSITIVE option was also the reason for the ObjectConverter class and the ObjectResolver interface. The ObjectResolver would have to be implemented by the user while case-sensitivity would already be handled by the general ObjectConverter class that would require no changes at all.

The "too much complexity" is due to this added functionality.

The code of the pull request has been in production for years and has never required any fix. And both of those options have been very useful, too.

The only "coupling with DAO" in my code is the comment "An ObjectResolver can be used to wrap a DAO." in the javadoc of my ObjectResolver. Anything else regarding DAO or repository is contained in the sample app and was meant as an example for "This is how you would include completion for your arbitrary business objects."... which I also said here but you just stopped the conversation at that point.

All of this gives me the impression that you didn't give my PR more than a casual glance before deciding that you'd rather implement something yourself.

I especially think that not including a sample application would have vastly increased my chance of getting this applied - which is just sad.

Regarding the documentation of org.springframework.shell.converters.ArrayConverter:

• the only non-private javadoc in that class is A converter that knows how to use other converters to create arrays of supported types..
• Spring Shell Documentation does not contain the word ArrayConverter.
• helloworld src was last updated two years ago. So it doesn't contain an ArrayConverter sample either.
• there is no changelog other than the commit history that I'd be aware of.
  • SHL-173 - Automatically support Converter<Foo[]> when there is a converter for Foo has not been updated since it was created. It's still Open/Unresolved. Which is kind of true since you are supporting Converter<Object[]> instead of Converter<Foo[]>.
  • 1.1.0 fixed that the application context was loaded twice during application startup. This wasn't mentioned anywhere. I found out about it be trying the new version.
  • 1.2.0 might include crucial fixes like that which I'm unaware of so it's probably a good idea to stay up-to-date.

I was obviously able to upgrade to 1.2.0 after refactoring my implementation into a different package to prevent the ArrayConverter name clash. But that is really not the point.

spring-shell is used by us pretty extensively and with great success.

Because of that, there have been several other things we've added in the meantime that could have been interesting for spring-shell in general. But because of the way this PR was first delayed and then killed, it's hard to argue why someone should invest the time to prepare a proper pull request.

This includes stuff like...

• a pretty crucial fix to the way the history is handling a large history file or a history file including very long lines, reducing startup of the app from >10min to seconds in that case.
• improved option to only keep a history with a certain number of entries.
• support for variables in commands, i.e. keeping the results of a previous command in a variable that can be used as an input for another command.

I hope the irony of having a project "at a minimal level of support" and not caring for third-party contributions is not lost on you.

While it certainly appears to be true that this project is "at a minimal level of support", this isn't mentioned anywhere either. Which means that the way this issue/PR was handled is assumed for springframework in its entirety.

And this isn't a theory but just a plain fact regarding myself and all of my coworkers.