jpos / jPOS

jPOS Project
http://jpos.org
GNU Affero General Public License v3.0
599 stars 458 forks source link

Basic support for string array auto-configuration. #520

Closed alcarraz closed 1 year ago

alcarraz commented 1 year ago

Proposal for a basic support for autoconfiguration of string array fields.

More complex logic could be added to support different ways of defining string arrays, or even collections of other types, in a single property, if desired, but this could do for now.

An unrelated fix for the doc is also included, that may not deserve a specific PR, and updating my mail address also.

If preferred, I can break this PR in 2 or 3.

ar commented 1 year ago

Good PR I was about to implement. It would be great to have support for int[] and long[] as well. Will add it as time permits.

alcarraz commented 1 year ago

Good PR I was about to implement. It would be great to have support for int[] and long[] as well. Will add it as time permits.

Thanks, I was thinking to add a property to the annotation for the separator, so it is read from a single property, I see two options for this:

  1. Two properties, a boolean and a char, the boolean saying if it's a single property, and the char, is a separator, with sane defaults.
  2. One String property as separator, where null means "multi-property" array, and not null, it sets the separator.

Each has its pros and cons, the first seems too verbose for such a simple thing. The later I particularly don't like, because I don't like to represent single chars as String, but on the other hand it would allow defining more than one separator.

If you want, we could discuss this in an issue or discussion, and if you want, I could implement it.

ar commented 1 year ago

Can you clarify with an example of both options? I'm not sure I'm quite following you.

alcarraz commented 1 year ago

Yes, sorry.

For a property definition like this:

<property name="array" value="one two three"/>

Option 1 would be something like this:

@Config(value="array", multi=false, separator=' ') 

Of course, the multi field could be single and have the opposite meaning, depending on what feels clearer.

Option 2:

@Config(value="array", separator=" ")

It could also be something like this:

@Config(value="array", separators=", ") //in this case the name should be plural to better indicate intention

It also has to be decided which should be the sane defaults.

ar commented 1 year ago

I don't think we want to create a DSL here. What I suggest for the time being is to support the equivalent of cfg.getInts() and cfg.getLongs().

alcarraz commented 1 year ago

I was thinking about supporting the common case of space and comma separated arrays and collections, like in CheckFields. It's to avoid the verbosity in the configuration of having a property tag for each item.