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

[#1904] LocalDateBasicUserType incorrectly converting to java.sql.Timestamp (Multiset fetching) #1905

Closed
wants to merge 1 commit into from

Conversation

dsarlo
Copy link
Contributor

@dsarlo dsarlo commented May 16, 2024

Description

  • Changed fromString method to convert date/date time strings to a LocalDate instead of to a java.sql.Timestamp first
  • Added a LocalDate field to DocumentForCollections entity and the MultiSet fetch view associated with said backing entity
  • Updated multiset fetch test case to use the LocalDate to make sure exception is no longer thrown and test passes

Related Issue

Fixes #1904

Motivation and Context

Allows us to safely use a LocalDate in an entity view fetched using the Multiset fetch strategy without having to use a custom BasicUserType to override the behavior.

@dsarlo
Copy link
Contributor Author

dsarlo commented May 16, 2024

@beikov We can also do something like this to be compatible with someone using a datetime db type for a LocalDate. Lmk if you want me to change it to this

    @Override
    public LocalDate fromString(CharSequence sequence) {
        String input = sequence.toString();

        try {
            return LocalDate.parse(input, DateTimeFormatter.ISO_LOCAL_DATE);
        } catch (DateTimeParseException e) {
            try {
                LocalDateTime dateTime = LocalDateTime.parse(input, DateTimeFormatter.ISO_LOCAL_DATE_TIME);
                return dateTime.toLocalDate();
            } catch (DateTimeParseException ex) {
                throw new IllegalArgumentException("Invalid date or date-time format");
            }
        }
    }

@dsarlo dsarlo changed the title [#1904] LocalDateBasicUserType incorrectly converting to java.sql.Timestamp [#1904] LocalDateBasicUserType incorrectly converting to java.sql.Timestamp (Multiset fetching) May 16, 2024
@beikov
Copy link
Member

beikov commented May 17, 2024

Could you please use construct a DateTimeFormatter as static variable, that has an optional time part?

@rajadilipkolli
Copy link

rajadilipkolli commented May 17, 2024

Hi @dsarlo , since you have worked on this. can you please extend the same to OffsetDateTimeBasicUserType as well. There as well its same issue.

Once check this Thread, #1864 (comment)

cc : @beikov

@dsarlo
Copy link
Contributor Author

dsarlo commented May 17, 2024

Thanks y'all! I’ll take a look and will make the changes in a bit

@dsarlo
Copy link
Contributor Author

dsarlo commented May 17, 2024

@rajadilipkolli I do not see an issue with OffsetDateTimeBasicUserType. When using an OffsetDateTime in an entity view to trigger it, it's parsed correctly without issue. If you can provide some insight into the issue with it, I can take a better look

@beikov Changes pushed

…ng to LocalDate

Update LocalDateBasicUserType.java

Revert "Update LocalDateBasicUserType.java"

This reverts commit 8a9d6d9.

Removed override - address code review items

Update entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/type/LocalDateBasicUserType.java

Co-authored-by: Christian Beikov <[email protected]>

Update entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/type/LocalDateBasicUserType.java

Co-authored-by: Christian Beikov <[email protected]>
beikov added a commit to beikov/blaze-persistence that referenced this pull request Jun 5, 2024
@beikov
Copy link
Member

beikov commented Jun 5, 2024

Thanks for your work. I'll try to get this through the finish line as part of #1911

@beikov beikov closed this Jun 5, 2024
beikov added a commit to beikov/blaze-persistence that referenced this pull request Jun 6, 2024
beikov pushed a commit to beikov/blaze-persistence that referenced this pull request Jun 6, 2024
beikov pushed a commit to beikov/blaze-persistence that referenced this pull request Jun 8, 2024
beikov pushed a commit to beikov/blaze-persistence that referenced this pull request Jun 8, 2024
beikov pushed a commit to beikov/blaze-persistence that referenced this pull request Jun 8, 2024
beikov pushed a commit that referenced this pull request Jun 8, 2024
beikov pushed a commit to beikov/blaze-persistence that referenced this pull request Nov 4, 2024
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.

Exception thrown when using LocalDate in an EntityView being fetched using the Multiset strategy
3 participants