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

Register Converters for Offset java.time types in Jsr310Converters #2681

Closed
wants to merge 2 commits into from

Conversation

jxblum
Copy link
Contributor

@jxblum jxblum commented Aug 17, 2023

See #2677

jxblum added 2 commits August 16, 2023 17:01
We now appropriately handle OffsetDateTime and OffsetTime the same as all other java.time types, supported as simple types on Spring application (persistent) entity classes.

Closes #2677
@jxblum jxblum added in: mapping Mapping and conversion infrastructure in: repository Repositories abstraction labels Aug 17, 2023
@jxblum jxblum changed the title Register Converters for Offset java.time types in JSR310Converters Register Converters for Offset java.time types in Jsr310Converters Aug 17, 2023
@jxblum jxblum added the type: bug A general bug label Aug 17, 2023
Comment on lines +54 to +65
new BinaryConverters.StringToBytesConverter(),
new BinaryConverters.BytesToStringConverter(),
new BinaryConverters.NumberToBytesConverter(),
new BinaryConverters.BytesToNumberConverterFactory(),
new BinaryConverters.EnumToBytesConverter(),
new BinaryConverters.BytesToEnumConverterFactory(),
new BinaryConverters.BooleanToBytesConverter(),
new BinaryConverters.BytesToBooleanConverter(),
new BinaryConverters.DateToBytesConverter(),
new BinaryConverters.BytesToDateConverter(),
new BinaryConverters.UuidToBytesConverter(),
new BinaryConverters.BytesToUuidConverter()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can drop the extra BinaryConverters. part here.

@@ -58,6 +61,7 @@ public abstract class Jsr310Converters {
}

List<Converter<?, ?>> converters = new ArrayList<>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be changed to List.of or initialized with an initial size.

@@ -296,4 +304,51 @@ public Duration convert(byte[] source) {
}
}

/**
* @author John Blum
* @see java.time.OffsetDateTime
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

though not a public class, having a @since tag can help with code forensic

@mp911de mp911de self-assigned this Aug 17, 2023
@mp911de mp911de removed the in: repository Repositories abstraction label Aug 17, 2023
@mp911de mp911de added this to the 3.0.9 (2022.0.9) milestone Aug 17, 2023
mp911de added a commit that referenced this pull request Aug 17, 2023
Replace qualified class name access of inner classes with simple names and imports.

Remove Java 8 guards. Extend supported temporal types in Jsr310Converters. Remove superfluous converter annotations.

Simplify tests.

See #2677
Original pull request: #2681
mp911de added a commit that referenced this pull request Aug 17, 2023
Replace qualified class name access of inner classes with simple names and imports.

Remove Java 8 guards. Extend supported temporal types in Jsr310Converters. Remove superfluous converter annotations.

Simplify tests.

See #2677
Original pull request: #2681
mp911de added a commit that referenced this pull request Aug 17, 2023
Replace qualified class name access of inner classes with simple names and imports.

Remove Java 8 guards. Extend supported temporal types in Jsr310Converters. Remove superfluous converter annotations.

Simplify tests.

See #2677
Original pull request: #2681
@mp911de
Copy link
Member

mp911de commented Aug 17, 2023

That's merged, polished, and backported now.

@mp911de mp911de closed this Aug 17, 2023
@mp911de mp911de deleted the issue/2677 branch August 17, 2023 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: mapping Mapping and conversion infrastructure type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jsr310Converters does not contain converters for OffsetTime and OffsetDateTime
3 participants