Support initialisation of ToMany collections using List.of() etc

[rbygrave]

• Add Support for List.of() & Collections.emptyList()
• Add Support for Set.of() & Collections.emptySet()
• Add Support for Map.of() & Collections.emptyMap()

For example:

@OneToMany(cascade = CascadeType.PERSIST)
List<Contact> contacts = List.of();

[rPraml]

My expectation is, that this should fail.
When initializing with List.of etc. the collection is immutable.
Can we transfer this information to the BeanCollection? (if agent detects immutable initialization, the replaced collection is also immutable from the perspective of the user - not for lazy loading)

BTW. There are other ways to initialize immutable lists:

• Collections.EMTY_LIST
• Collections.unmodifiableList(new ArrsyList())
• Arrays.asList(...)

I think, the agent should fail or at least warn, if an unknown initialization is detectef

[rbygrave]

My expectation is, that this should fail.

That is reasonable. My expectation was that people wouldn't use List.of() at all ...

Can we transfer this information to the BeanCollection?

The initialisation for OneToMany and ManyToMany is removed. So for example, if we detect new ArrayList<>() initialisation for a OneToMany - that bytecode is removed so it is as if it wasn't there at all.

So this PR was done such that initialisation via List.of() ... was detected and removed as if it wasn't there at all.

That is, the only reasons why we'd choose to initialise OneToMany collections in such a way is so that (A) the fields can be final and (B) it gives the impression to people reading the code that the collection will not be null.
... noting the for (B) this is only an impression because ebean will make sure the collection is not null regardless [well except for the new immutable read only stuff that isn't in yet and will throw an exception ... so also there no application code will see a null OneToMany collection].

---

So that all said, I've pushed another PR just now that detects when initialisation code for OneToMany or ManyToMany isn't supported and throws a new EnhancementException [and for maven we will fail the build at enhancement due when this is detected].

I think, the agent should fail or at least warn, if an unknown initialisation is detect

Yes. I think I've just managed to do that with the latest commit pushed here plus a change to the maven plugin to detect and fail the build for that case.

The outstanding question I think is, should we support List.of() and Collections.emptyList() [where support means that this bytecode is detected and removed just like new ArrayList<>() is].

[rPraml]

That is reasonable. My expectation was that people wouldn't use List.of() at all ...

I would never have thought of that ;)

We would need the following use cases:

• null / new ArrayList() for normal lists
• a distinction between unsorted sets and sorted sets/maps (we initialize them with LinkedHashSet/LinkedHashMap)

The outstanding question I think is, should we support List.of() and Collections.emptyList()

if we decide to do, I would expect, that the BeanList/BeanSet etc. is also readonly. So BeanList needs a readOnly flag and the agent must call the new constructor new BeanList(underlyingList, true)

I can imagine use cases, where I want to protect the modification of that list by someone else.

but this is more theoretical, so it would be also OK to throw an error in the enhancer

[rbygrave]

a distinction between unsorted sets and sorted sets/maps

I have no intention of ebean supporting unsorted sets/maps. I don't think that is a need / requirement.

I would expect, that the BeanList/BeanSet etc. is also readonly.

I don't think we can easily do that at this point. I don't want to try to get that clever unless we have to.

My suspicion is that I don't think people are thinking deeply about this. I suspect the use of List.of() was more just a bit of an accident.

[rbygrave]

Reviewing the JPA spec, the initialisation of OneToMany and ManyToMany is vague:

"Collection-valued persistent fields and properties must be defined in terms of one of the following collection-valued interfaces regardless of whether the entity class otherwise adheres to the JavaBeans method conventions noted above and whether field or property access is used: java.util.Collection, java.util.Set, java.util.List [3], java.util.Map. The collection implementation type may be used by the application to initialize fields or properties before the entity is made persistent."

Noting that stack overflow has - https://stackoverflow.com/questions/20715143/to-initialize-or-not-initialize-jpa-relationship-mappings

At this stage, it feels like the safest option would be to not support List.of() and Collections.emptyList() etc as long as we can explicitly fail the enhancement. Noting that there might have "holes" in this detection like an application that ONLY using javaagent style enhancement could miss an error going to system out [but the entity would not be enhanced so it would not be useable as an entity - it's just the error might get misleading].

[rPraml]

This looks good to me now. We should not add too much complexity

[rbygrave]

Ok cool - I think we can roll with this.

[rbygrave]

Also needs PR #221 to capture some non valid cases