OneToMany does not support initialisation via List.of() - Was - Fetching multiple ToMany eagerly doesn't work on transitive associations

[apflieger]

Underlying issue here:

@OneToMany
private List<Attendance> attendances = List.of(); // we can't use List.of() 

A collection initialised with List.of() isn't supported. It should be left uninitialised or initialised as new ArrayList<>() [which are treated exactly the same, ebean enhancement removes that initialisation code as it must control the initialisation of collections to support orphanRemoval and ManyToMany adds/removes]

---

Hi,

Thanks a lot for your work, that's always worth saying.

Context

I am trying to load an entity that has multiple ToMany relations, but transitively, in a single EBean query. The model look like this:

class Payment {
    @ManyToOne
    Ticket ticket;
}

class Ticket {
    @OneToMany
   List<Payment> payments;

    @OneToMany
   List<Attendance> attendances;
}

I am fetching Payments entities with a FetchGroup that queries the two paths ticket.payments and ticket.attendances.

Expected behavior

I expect to have one query with a join for the first relation, and a secondary query for the second.

Actual behavior

What I see is that secondary queries are not executed, everything is then lazy loaded one row at a time.

I didn't setup a reproduction case, let me know if that's necessary. But I spent some time to debug and I landed here
At this point we are in a secondary query, but it got skipped by the condition loadBuffer.size() > 0, this buffer is empty at this stage. I couldn't understand why exactly. What I found is that this buffer is created based on Bean descriptions in this code (ctx.register calls DLoadManyContext.createBuffer), but the bean description of the query (root entity Payment) doesn't contain many assoc, so localDesc.propsMany() is empty on line 303

Additional note

I can't affirm it's a regression because we changed our model and queries, but I feel like this worked in the past.

Cheerz and long live EBean ✌️

[rbygrave]

Well, for example the following test passes and produces the correct sql, where shipments and details are OneToMany relationships from order.

  @Test
  void test() {
    ResetBasicData.reset();
    LoggedSql.start();

    Query<Order> q = DB.find(Order.class)
      .fetch("shipments")
      .fetch("details")
      .fetch("details.product")
      .fetch("customer")
      .where().gt("id", 0).query();

    List<Order> list = q.findList();
    assertThat(list).isNotEmpty();

    List<String> sql = LoggedSql.stop();
    assertThat(sql).hasSize(2);
    assertThat(sql.get(0)).contains("from o_order t0 join o_customer t1 on t1.id = t0.kcustomer_id left join or_order_ship t2 on t2.order_id = t0.id where");
    assertThat(sql.get(1)).contains("from o_order_detail t0 left join o_product t1 on t1.id = t0.product_id");
  }

What version of ebean are you using?
What exactly does the query look like?
Perhaps you can git clone and modify TestQueryMultiManyOrder [the above test] to reproduce the issue?

What I see is that secondary queries are not executed

Note that secondary queries will not execute if there are actually no "parent" beans. In the example above, if there are no order rows, then the secondary queries will not execute.

[apflieger]

My guess is that here, shipments and products are direct associations of Order. So these toMany are registered on the BeanDescription, which makes the buffer created properly.
I'll try to reproduce the case

[rbygrave]

Cool. Note that there was also:

• What version of ebean are you using?
• What exactly does the query look like?

[apflieger]

I am on ebean 15.9.0.
I managed to reproduce the problem here with a minimal test #3581

I found the origin of the issue, which is completely different from what I described originally and has nothing to do with transitive associations. The secondary query is prevented here still, but this buffer is actually empty because of what happen here. The ref object is non null, so ctx.register is not executed. I found that it comes from the way I initialize collection fields. I do that

class Ticket {
    @OneToMany(cascade = CascadeType.PERSIST)
    private List<Payment> payments = new ArrayList<>(); // This initialization has no impact as it is fetched by the main query

    @OneToMany
    private List<Attendance> attendances = List.of();
}

And the ref object is null because getValue here returns the List.of() instance. What is surprising is that when I init the field with new ArrayList() instead, it works.

My pull request demonstrates that.

I'm not event sure if field initialization is supported this way? Originally we didn't have these initialization, but an ebean upgrade forced me to put them. It was from 13.23.1 to 15.5.0

[rbygrave]

List.of()

That returns an immutable empty list. So no that isn't supported. In short, collections don't work the way you are thinking that they work - they MUST be initialised by ebean.

Effectively the bug here is that ebean enhancement isn't expecting initialisation via List.of() (and it probably should detect that bytecode and remove it - see below).

payments = new ArrayList<>();

Note that ebean enhancement detects this case of initialising a collection with a new ArrayList<>() and actually removes that bytecode. This is because ultimately these collections need to detect additions/removals for ManyToMany and orphanRemoval=true cases. So ebean enhancement removes that collection initialisation [and the collections that do get used instead are ebean implementations for List, Set, Map being BeanList, BeanSet, BeanMap].

That is, ebean must control the initialisation of the actual collection instances for ToMany collections (and those collections are never presented as null to user code - you'll never see a null ToMany collection with or without that initialisation).

Originally we didn't have these initialization, but an ebean upgrade forced me to put them.

This doesn't make sense to me in that there is no need to initialise ToMany collections for ebean and in fact ebean enhancement will look to detect that initialisation bytecode and remove it.

So it's not clear why you say "upgrade forced me to put them" as that should never be the case, we can equally leave the ToMany collections uninitialised.

[apflieger]

So it's not clear why you say "upgrade forced me to put them" as that should never be the case, we can equally leave the ToMany collections uninitialised.

I just removed these initializations, everything seems to work. I don't know what forced me back then 🤷.

[rbygrave]

Ok cool, that's great.

The PR ebean-orm/ebean-agent#220 ... changes the ebean-agent to support detection and removal of List.of(), Collections.emptyList() and similar for Set and Map. This change to the ebean-agent should be considered the "fix" for this issue.

The workaround in the meantime is to either remove the List.of() initialisation altogether or replace it with new ArrayList<>(); ... which is detected and removed].

[rbygrave]

Note: The ebean-agent has been modified to support List.of() but also detect initialisation of OneToMany and ManyToMany that isn't supported and fail [throw error, log error, not enhance the bean] for those cases.

That is, this bug is in the ebean-agent enhancement.