-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
HHH-19017: Address ClassCastException for PersistentAttributeInterceptable #9605
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jimsimon-wk! Just left a few minor comments, but other than that your change looks good to me.
...main/java/org/hibernate/sql/results/graph/entity/internal/EntityDelayedFetchInitializer.java
Outdated
Show resolved
Hide resolved
scope.inTransaction(session -> { | ||
// Create company | ||
Company company = new Company(); | ||
company.id = 1L; | ||
company.name = "Hibernate"; | ||
session.persist(company); | ||
|
||
// Create project | ||
Project project = new Project(); | ||
project.id = 1L; | ||
session.persist(project); | ||
|
||
// Create employee | ||
Employee employee = new Employee(); | ||
employee.id = 1L; | ||
employee.company = company; | ||
employee.projects = List.of(project); | ||
session.persist(employee); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually we separate preparing the data for tests in a @BeforeAll
method. Also an @AfterAll
that cleans after the test would be great, you can simply add this:
@AfterAll
public void tearDown(SessionFactoryScope scope) {
scope.getSessionFactory().getSchemaManager().truncateMappedObjects();
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed 5b54685
.getSingleResult(); | ||
}); | ||
|
||
// No assertions here, because this test trigger an exception with Hibernate 6.6.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just add a couple assertions verifying the returned Employee
instance contains what you expect (an initialized projects
collection, and the correct company
association), they're not fundamental for what you're trying to fix but still they ensure consistency in the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed 5b54685
Thanks for your pull request! This pull request appears to follow the contribution rules. › This message was automatically generated. |
cc50bcb
to
620a985
Compare
620a985
to
ca7c629
Compare
Thanks for the helpful review @mbladel! I thought I had imported the codestyles to IntelliJ, but I must have forgotten to apply formatting. This moved things around a little in the unit test too |
A
ClassCastException
is thrown when trying to resolve instances without bytecode enhancement enabled under these preconditions:Related Change in Hibernate 6.6
Reproducer
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.
https://hibernate.atlassian.net/browse/HHH-19107
https://hibernate.atlassian.net/browse/HHH-19017