Skip to content

Conversation

tim-aero
Copy link
Contributor

Added new mapper for Java's LocalDate and LocalDateTime as well as changing the TypeUtils

@tim-aero tim-aero requested a review from reugn January 10, 2023 22:17
Copy link
Member

@reugn reugn left a comment

Choose a reason for hiding this comment

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

  • Optimize imports
  • It will be good to have integration tests for the mappers

if (value == null) {
return null;
}
return Date.valueOf((LocalDate) value).getTime();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return Date.valueOf((LocalDate) value).getTime();
return ((LocalDate) value).toEpochDay();

return null;
}
long longValue = (Long) value;
return new Date(longValue).toLocalDate();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return new Date(longValue).toLocalDate();
return LocalDate.ofEpochDay(longValue);

if (value == null) {
return null;
}
return Date.from(((LocalDateTime) value).toInstant(ZoneOffset.UTC)).getTime();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return Date.from(((LocalDateTime) value).toInstant(ZoneOffset.UTC)).getTime();
return ((LocalDateTime) value).toInstant(ZoneOffset.UTC).toEpochMilli();

- Added a unit test for LocalDateMapper and LocalDateTimeMapper
- Changed LocalDateMapper to use EpochDays
- Changed LocalDateTimeMapper to preserve full precision of the
LocalDateTime down the nanosecond level.
Copy link
Member

@reugn reugn left a comment

Choose a reason for hiding this comment

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

  • Consider implementing a LocalTime mapper
  • Code format

Added a mapper for LocalTime to round out Java 8 date / time classes.
Included a unit test for them.
@tim-aero tim-aero requested a review from reugn January 16, 2023 23:40
@reugn reugn changed the title Added support for LocalDate and LocalDateTime Add mappers for java.time classes Jan 17, 2023
@reugn reugn merged commit 9bbf9d8 into aerospike:main Jan 19, 2023
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

Successfully merging this pull request may close these issues.

3 participants