deltaspike
deltaspike copied to clipboard
Support for Additional Converters by Default
Adds default converts for the following types:
- java.awt.Color
- java.awt.Dimension
- java.awt.Point
- java.io.File
- java.net.InetAddress
- java.net.URI
- java.net.URL
- java.time.Duration
- java.time.Instant
- java.time.Period
- java.util.Locale
- java.util.UUID
- java.util.regex.Pattern
Edit: Originally a request to add a plugin to the docs, but made this a contribution instead.
In general i wonder why dont you contribute directly to DeltaSpike?
I could try to do that if it's alright. I have no problem with committing code in the DeltaSpike project, in fact, I'd prefer it.
I just kept it separate as I needed it for another project and didn't feel familiar enough with the DeltaSpike project internally to feel confident contributing directly.
If you've seen them and believe they could, and should be contributed into this repository, l gladly give it a shot to move them over.
I've moved everything from the proposed plugin into the DeltaSpike Core Impl module.
It includes implementations for several standard Java types, tests for each converter, and a small documentation update to mention the additional supported types.
Does this look alright so far for a PR?
looks very good to me! @struberg can we merge it?
Since opening this PR, I've noticed Apache BeanUtils has many converters similar to how this project does. :thinking:
Would it be feasible to have a general "converters" project, and have both DeltaSpike and BeanUtils depend on it?
https://github.com/apache/commons-beanutils/tree/master/src/main/java/org/apache/commons/beanutils2/converters
Not sure if both projects are interessing in such a shared codebase. Couldnt we reuse beanutils here? cc @struberg
If possible, it could be nice to get an answer on this. I'm hoping the pull request to commons-beanutils could be merged soon.
- If DeltaSpike may depend on BeanUtils: we can close this, and I may look if I can add BeanUtils in a separate PR here.
- If DeltaSpike won't depend on BeanUtils: I'll rebase/update this to make improvements or match feedback from BeanUtils.
Hi Seth! I'd rather not depend on BeanUtils. The less dependencies we do have, the better it is imo. The code for picking up the converters is rather trivial.
That said, what do you think about doing something similar than we did when implementing mp-config over in Geronimo: implicit converters! https://github.com/apache/geronimo-config/blob/ConfigJSR/impl/src/main/java/org/apache/geronimo/config/converters/ImplicitConverter.java
I've also added this to the ConfigJSR proposal: https://github.com/eclipse/ConfigJSR/blob/master/spec/src/main/asciidoc/converters.asciidoc#implicit-converters With that we'd need to add Converters only for those classes where the implicit converter rules do not already apply. Wdyt?
Hey! Understood, I'll squash this PR and match the improvements suggested from BeanUtils. Even if this won't be merged as is, the logic should still be updated anyway.
Meanwhile, having implicit converters seems cool.
This PR includes 5 converters that would be made redundant, assuming we stuck with the current implementations.
-
Duration (
Duration#parse
) -
File (
File::new
) -
Instant (
Instant#parse
) -
Period (
Period#parse
) -
URI (
URI::new
)
Just a note, but something that could've been implicit, but doesn't meet the current spec is #fromString(java.lang.String)
?
But only the following have it in the standard library:
Just finished cleaning this up, a summary of the changes:
- Squashed the commits.
- Reduced the verbosity of the docs/errors. (Semantics are the same, they were just really wordy.)
- Used more concise syntax for testing exceptions. (
@Test(expected = {}
) (Comment) - Updated
@since 1.9.5
to@since 1.9.6
since 1.9.5 has already been released. -
Duration
,Instant
, andPeriod
no longer acceptLong
values by default. (Comment) - Updated tests for
URL
to compare theURI
instead to avoid depending on an internet connection. - Allow British and American spellings for
Color
. (Comment)