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

Micronaut Data JDBC problem with bidirectional ONE_TO_MANY relation, doesn't initialize parent reference in all children #1054

Closed
alejandropg opened this issue Jun 14, 2021 · 11 comments · Fixed by #1072

Comments

@alejandropg
Copy link

I'm using Micronaut Data JDBC to map two entities, a parent with children, with bidirectional ONE_TO_MANY relation.

But when I retrieve the parent, one of their children is initialized with null reference to the parent. The other children are correctly pointing to the parent.

Steps to Reproduce

Entire source code in: https://github.com/alejandropg/micronaut-data-jdbc-demo-one-to-many

@MappedEntity
data class Parent(
    val name: String,

    @Relation(value = Relation.Kind.ONE_TO_MANY, mappedBy = "parent", cascade = [Relation.Cascade.ALL])
    val children: List<Child>,

    @field:Id @GeneratedValue
    val id: Int? = null
)
@MappedEntity
data class Child(
    val name: String,

    @Relation(Relation.Kind.MANY_TO_ONE)
    val parent: Parent? = null,

    @field:Id @GeneratedValue val id: Int? = null
) {

    // Override to avoid infinite recursion when navigate to parent
    override fun equals(other: Any?): Boolean ...
    override fun hashCode(): Int ...
    override fun toString(): String ...
}
@JdbcRepository(dialect = Dialect.H2)
abstract class ParentRepository : CrudRepository<Parent, Int> {

    @Join(value = "children", type = Join.Type.FETCH)
    abstract override fun findById(id: Int): Optional<Parent>
}
    @Test
    internal fun `save parent`() {
        val children = mutableListOf<Child>()
        val parent = Parent("parent", children)
        children.addAll(
            arrayOf(
                Child("A", parent),
                Child("B", parent),
                Child("C", parent)
            )
        )
        val saved = repository.save(parent)
        println(saved)

        val found = repository.findById(saved.id!!).get()
        println(found)
        ...

Expected Behaviour

When I call findById I expect to retrieve the parent with the list of their children and each one of these children pointing to the parent.

Actual Behaviour

When you retrieve the parent, one of their children is pointing to a null parent.

micronaut-data-jdbc-demo-one-to-many_–_ParentRepositoryTest_kt

Environment Information

  • Operating System: macOS Big Sur 11.4 (Intel)
  • Micronaut Version: 2.5.5
  • Micronaut Data Version: 2.4.4
  • Kotlin Version: 1.5.10
  • JDK Version: 11

Example Application

@PiotrBaczkowski
Copy link

PiotrBaczkowski commented Jul 2, 2021

But won't there be infinite loop of parent- > children ->parent-> children?

@alejandropg
Copy link
Author

Look that the null problem is just with the first of the children, not with the others.

And also take account that, for now, Micronaut creates two instances of the Parent class:

  • One instance is used as parent attribute in the children
  • and the other instance, is created and populated with the list of children and returned to you as the retrieved object.

You can see that in the attached image. Look that the found object is Parent@5925 and the parent attribute in the children references the instance Parent@1837.

The "two instances" issue is not really a problem, and maybe could be optimiced to create just one instance, but this is not really the problem with the null reference.

@graemerocher
Copy link
Contributor

it is not possible to optimise it to create just one since one is created by you, but in order to provide an id to the instance after it is saved we have to use a copy constructor to instantiate a new instance since the id is not mutable in your Parent data class

@alejandropg
Copy link
Author

Yes, and this is not the problem, just a comment to answer @PiotrBaczkowski

The issue I want to comment is that when you retrieve the one to many relation, the first child is initialised with a null parent, while the other children are initialised with a correct parent.

@graemerocher
Copy link
Contributor

yes that behaviour appears to be a bug

@dstepanov
Copy link
Contributor

I will look at it

@alejandropg
Copy link
Author

Thanks a lot 🙏

@dstepanov
Copy link
Contributor

@alejandropg I have fixed the missing parent, but your test tests a bit more:

  • we do not support deleting a child record from the collection as we don't have a way to tell that it was there before, we would need to implement executing one extra delete all children where id not in and is matching parent for all updates...
  • it looks like that we don't propagate the update for relationships with mappedBy. That kind of mapping defines the owner of the association, in this case, it's a child, but I'm not sure if it should be updating fields. Would be nice to know what is Hibernate doing in this case.

@alejandropg
Copy link
Author

Yes, I'm sorry because I should have alerted you before about I was playing with more things and I had more tests 😓

About the delete and update that you mentioned, delete is ok, but the update, I think that maybe the documentation is not clear because it looks like the cascade update it's certainly supported: https://micronaut-projects.github.io/micronaut-data/latest/guide/#sqlAssociationMapping

Cascade.UPDATE Associated entity or entities are going to be updated when owning entity is updated

Isn't it? Or sure I have not understanding some other point, sorry 😅

So @dstepanov, the update should works or not? By your comment I think that it should but right now it isn't, right?

@dstepanov
Copy link
Contributor

I would expect the update to work, you can create a new issue so it's not lost.

And also take account that, for now, Micronaut creates two instances of the Parent class:

It's a problem when you have bi-directional relation and immutable classes, there might be a good idea to allow to map some kind of Provider<Parent>/Supplier<Parent>/some Kotlin equivalent which can be internally mutable and to allow us to set the parent instance later.

@alejandropg
Copy link
Author

Thanks @dstepanov 👍 Sure, no problem, I will add a new issue with the "update", and will prepare the example code with the correct tests 👌

With the two parent "problem" I'm sure that it's not an easy problem. With Kotlin, we used to create the parent first, passing a mutable list that the parent stores in a List property (the non mutable interface). Then we create the children one by one, passing to them the instance of the parent, and then adding the children to the mutable list.

So, the effect for the parent's observer is that the parent is immutable because it can see only the List interface.

But I suppose that is not ease to do all this stuff automagically, discovering the constructors, the properties... 😅

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 a pull request may close this issue.

4 participants