[hibernate-dev] HHH-11898: more "empty" composite issues

[Gail Badner]

After thinking about this more, I realize there are some very serious
issues if Hibernate:

* allows composites with primitive or initialized (non-null) attributes to
be instantiated when hibernate.create_empty_composites.enabled=true
* assumes that an empty composite with primitive values or initialized
(non-null) values is equivalent to null, regardless of how
hibernate.create_empty_composites.enabled is mapped.

If you have any doubts, please take a look at a test case that illustrates
the issues. [1]

The question now is what to do about it.

Here are some alternatives for what can be done when
hibernate.create_empty_composites.enabled=true:

A) Throw a MappingException if any composites have a primitive or
initialized (non-null) attribute;
Pros:
* this is an easy check that will keep data from being corrupted.
Cons:
* having primitive values in a composite is very common, so the "empty
composites" features will likely be unusable without having to make
(possibly extensive) updates to the application;

B) For each composite that contains a primitive or initialized (non-null)
attribute, log a warning and override
ComponentType#createEmptyCompositesEnabled to be false so empty composites
will not be created for that composite.
Pros:
* this is an easy check that will keep data from being corrupted.
Cons:
* this would likely result in annoying warnings that people may end up
ignoring;
* if a primitive attribute is added to a composite that didn't have any
primitive attributes, a new warning could turn up and not be noticed; NPEs
could be thrown because an attribute is null when the application expects
it to be an empty composite.

C) Allow a custom strategy to be configured that will indicate whether
empty composites should be supported for a particular ComponentType, using,
for example:

hibernate.create_empty_composites_strategy=<strategy>, where <strategy> can
be:
* a fully-qualified class name;
* instance of EmptyCompositeStrategy.

public interface EmptyCompositeStrategy {
    /**
     * Should a composite/embeddable be instantiated when a null attribute
     * of the specified type is read?
     *
     * @return true if Hibernate should instantiate a composite/embeddable
     * object when a null composite attribute is read.
     */
    boolean supportsEmptyComposite(ComponentType componentType);

    /**
     * Is the specified value an empty composite that should be considered
     * equal to <code>null</code>?
     *
     * @return true if the specified value is an empty composite.
     */
    boolean isEmptyComposite(ComponentType componentType, Object value,
Object component, SessionFactoryImplementor factory);
}

It would be nice to be able to provide the composite role to the
EmptyCompositeStrategy so that the same composite class can be treated
differently depending on context (e.g, owner entity, composite in a
particular composite, map key, collection element). The role is not always
available from ComponentMetamodel. I'm not sure how difficult it would be
to get it set properly.

Hibernate could provide StandardEmptyCompositeStrategy, which would
implement A) above, and could be used when
hibernate.create_empty_composites_strategy=true.

Pros:
* straightforward to implement;
* puts the onus on the strategy to determine:
** which composite classes should be instantiated when a null attribute is
read;
** if composites with primitive or non-null initialized values should be
instantiated; this should only be done when the composite is not used as an
ID or foreign key (warn if it is?);
* allows the the strategy to take shortcuts when determining if a composite
value is empty; e.g., excluding random/date values that are initialized by
the constructor.
Cons:
* the strategy could be complicated if there are lots of composites with
different requirements;
* context would be difficult to determine currently because the role is not
available; all uses of the same composite class would need to be treated
the same way.

D) Provide a way to opt-in when
hibernate.create_empty_composites_strategy=false, or to opt-out when
hibernate.create_empty_composites_strategy=false.

This could be done using a new annotation that provides a strategy for the
annotated @Embedded value.

Pros:
* the context of the annotation would be available so the way a composite
is treated can differ depending on context.
* may simplify using different strategies for different composites or
different contexts of a composite class;
Con:
* providing this level of flexibility may not be warranted for this feature.

My preference is C).

Comments or opinions?

Thanks,
Gail

[1]
https://github.com/hibernate/hibernate-orm/pull/1993/commits/329354bfa4bd86190252f1d5a7019f8b06c21b17

On Thu, Aug 10, 2017 at 5:46 PM, Gail Badner <gbadner at redhat.com> wrote:

> I realized that ComponentType#isEqual as well as #isSame, #compare,
> #isDirty, and #isModified do not treat empty composites as equivalent to
> null in the following cases:
>
> 1) the composite has a primitive attribute;
> 2) the composite has a singular attribute that gets initialized to a
> non-null (or non-default primitive) value when the composite is constructed;
> 3) the composite has a plural attribute.
>
> It would be straightforward to compare a primitive value in a composite
> value with the default for the primitive type (e.g, comparing an int
> property value with 0 instead of null).
>
> I think it is reasonable to have Hibernate assume that a composite with
> Object attributes set to null and primitive values set to its default value
> to be considered an empty composite.
>
> I am a little concerned that a primitive value that happens to be set to
> the default could be a "real" value intended to be persisted, so I would
> like to propose logging a warning when hibernate.create_empty_composites.enabled=true
> and a composite has a primitive value. The message would mention the
> ambiguity and recommend using a non-primitive attribute instead.
>
> Regarding 2), here are some examples of a singular attribute initialized
> to a non-null (or non-default primitive) value when the composite is
> constructed
>
> Examples:
> boolean isAvailable = true;
> Integer  length = -1;
> Date created = new Date();
> double random = Math.random();
>
> IMO, Hibernate should throw an exception when hibernate.create_empty_composites.enabled=true,
> because empty composites, by definition, should have attributes that
> correspond to null columns.
>
> At the very least, Hibernate should not automatically inject an
> instantiated composite with initialized values when composite columns are
> all null.
>
> Regarding 3), if a composite contains a plural attribute, Hibernate
> automatically injects a PersistentCollection into the empty composite.
> ComponentType#isEqual, #isSame, #compare, #isDirty, and #isModified do not
> take this into account when comparing the empty collection value with null.
>
> IMO, this should be fixed. ComponentType#isEqual, #isSame, #compare,
> #isDirty, and #isModified, should all assume an empty collection is
> equivalent to null.
>
> I suppose it could be helpful to add support for custom strategies for
> determining if an instantiated composite is empty. For example, a strategy
> could disregard some attributes (e.g., dates, random numbers), or have it's
> own criteria for what an empty composite is. The default would be check all
> attributes in the composite equal to null, primitive default, or empty
> collection. I have no plans to pursue at this point though.
>
> Anyone have any comments on any of this?
>
> Thanks,
> Gail
>
>