mutability of EntityGraphs #771
Description
The EntityManager API makes reference to an undefined notion of "mutability" of an EntityGraph. The only real impact this has on the APIs is the bifurcation here:
/**
* Obtain a mutable copy of a named {@link EntityGraph}, or
* return null if there is no entity graph with the given
* name.
*/
EntityGraph<?> createEntityGraph(String graphName);
/**
* Obtain a named {@link EntityGraph}. The returned instance
* of {@code EntityGraph} should be considered immutable.
*/
EntityGraph<?> getEntityGraph(String graphName);It's not very clear what "should be considered immutable" means. Is it an actually immutable or not? There's at least two possible interpretations:
- is the returned object is a direct reference to the internal state held by the
EntityManagerFactory, and so should not be mutated, or - does mutation of the returned object lead to a runtime exception? If so, what exception type is thrown?
The first interpretation is potentially quite dangerous, but the second isn't much better, since it's completely lacking in type safety!
Worse, the operation getEntityGraphs(Class) fails to mention whether the objects it returns are mutable or immutable. One imagines that the intention was that it returns immutable graphs, since it's called "get" not "create", but this is the sort of thing you have to write down!
But, in fact, getEntityGraph() is completely unnecessary. Doing some mind-reading here, I imagine that the concern was that createEntityGraph() is required to return a mutable copy of the named graph, and that the copying process might be expensive (it's unclear that this is true for typical graphs, but whatever). But this presupposes an especially unimaginative implementation! One could always have createEntityGraph() return a proxy object for a data structure which does copy-on-write.
I therefore think that we should deprecate one of these methods. Since I imagine that getEntityGraph() is more commonly used than createEntityGraph(), and because the overload of createEntityGraph() is at least a bit confusing, I'm more inclined toward deprecating createEntityGraph(), but I could go either way.
Taking that path, my proposal would be:
- eliminate the notion of a "considered immutable" graph, since no such notion is realized in a type safe way
- deprecate
createEntityGraph(String) - add
getEntityGraph(Class,String)as previously proposed in EntityManager.getEntityGraph(Class,String) #764.
I'm also still in favor of adding createEntityGraph() and getNamedEntityGraphs() to EntityType for use via the static metamodel.