users@jpa-spec.java.net

[jpa-spec users] Re: [jsr338-experts] Re: mapping conversion

From: Oliver Gierke <ogierke_at_vmware.com>
Date: Tue, 17 Jan 2012 07:33:49 -0800

Am 16.01.2012 um 21:33 schrieb michael keith:

>> Point taken. Do the types of the properties to be converted need to be known as JPA-special types (@Entity, @MappedSuperclass) at all then? The availability of a converter of property type X to natively mappable "column" type Y pretty much hands responsibility of mapping the object, right?
> Not sure what you mean. A converter converts only a single attribute in
> an entity, mapped superclass, or embeddable, not the entity/mapped
> superclass/embeddable itself.

That was more of a question actually. Let's say I have a

@Entity
class Wrapper {

  Wrapped wrapped;
}

class Wrapped { … }

class MyConverter implements EntityAttributeConverter<Wrapped, String> {

}

This should be enough, right? The Wrapped class doesn't have to be annotated beyond that, does it?

>>> Yes, they would be similar to EntityListeners in the fact that they would be instantiated by the provider. Injection is not as useful, though, since the point of converters is simply to transform attribute state from one form to another, not to perform domain operations. That doesn't mean we can't allow it, it's just not a priority IMO.
>> I didn't think of injecting an EntityManager or other sort of application component but rather of a way to configure a converter implementation by some means, e.g. configuring a date pattern a Date object shall be read and written into. I don't think it's a good idea to defer this question to a later stage as it requires quite some changes to the API and implementations if introduced later.
> Is a date pattern something that is unique to the converter and that
> could be defined in some static state within it?
> I guess it's possible that you might need some externally defined state
> in order to do the conversion, it just feels like making converters
> something like CDI beans might be overkill.

Assume the following:

class MyDateConverter implements EntityAttributeConverter<Date, String> {

  private final String format;

  public MyDateConverter(String format) {
    this.format = format;
  }

  public String toDatabaseValue(Date date) {
    // use format to create the string
  }

  // … same for reading
}

How would I get this converter working without a factory in between? I am fine if we go for allowing users registering converter instances but registering class names and requiring an empty constructor to create instances by reflection feels so 2000 when you had to teach people the values of DI. I also don't necessarily vote for deep CDI integration. The default mechanism might be exactly that (reflection, no-arg constructor) but please allow getting more detailed control about converter instantiation. Working with EntityListeners (which are stuck to that model) is usually either not gaining you much (if you stick to the standard instantiation mechanism, no injection available) or very complex (including AspectJ).

>> I also think that the instance creation of user defined extensions (what the EntityAttributeConverter actually is) should not be hardcoded into the persistence provider.
> Converter implementation classes would not be hardcoded into the
> provider any more than entity classes or any other user class would. The
> provider discovers the converter class just like it does entity classes,
> listeners, or other user-defined classes, and instantiates them
> reflectively on demand. No hardcoding necessary.

Nono, I referred to the *instance creation* (requiring a no-arg constructor to be able to instantiate the converter by reflection) being hard coded into the persistence providers. That's how it works with EntityListeners as described above. Doing so effectively turns persistence providers into factories for components like EntityListeners, AttributeConverters etc. and usually don't permit the flexibility necessary (as it's not their responsibility to).

Hibernate e.g. allows EventListener instances being registered which is a much more powerful approach.

>> Other comments on Mike's suggestions regarding converter annotations and EntityManagerFactory API see below.
>>
>> Am 16.01.2012 um 06:05 schrieb Bernd Müller:
>>
>>> I see a problem: the name of the converter is unique in the pu but
>>> the usage mimics locality for an entity.
>>>
>>> In JSF there is also a converter annotation called FacesConverter
>>> doing similar things. However it is used to annotate the converter (as
>>> the name implies ?)
>>>
>>> The refactored example:
>>>
>>> @Converter("booleanToInteger")
>>> public class BooleanToIntegerConverter implements
>>> ...
>>> }
>>>
>>> @Entity
>>> public class Employee {
>>> @Id long id;
>>> @Convert("booleanToInteger")
>>> boolean fullTime;
>>> ...
>>> }
>> +1. Although there's one caveat to this. As the flip switch to define whether a converter is a global one or not is now tied to the implementation (as it resides in the annotation) the implementation cannot be reused for the exact opposite use case. Besides this flag the @Converter
> I think it's reasonable to state declaratively in the converter whether
> it is a global conversion or not. That is probably where you would want
> to do it. If you don't want to put it there, though, you could always
> put it in an XML entry instead to decouple it from the code entirely.

But imagine a library like JodaTime coming up with implementations of EntityAttributeConverter. If the global-non-global config is done at the converter implementation they already have to do it. This would result in the need to annotate all JodaTime properties with @Convert("foo") or declare em in XML. If it's global by default, I just register the implementations with the EMF and opt-out at properties I don't want to have converted with the default global one. I even might want to have two different instances of the implementation class using different date formats: one global one plus one only used for a single property.

I think we have to consider the converters not only being written alongside the application but rather the type they convert. Thus they can be part of a library (such as JodaTime) and thus we should have as little configuration as possible at the implementation class.

>> annotation is just what @Named is. So I wonder if it might make sense to simply consider managed beans of a given type (EntityAttributeConverter<X, Y>) as converters? But that might be an implementation detail of the EntityAttributeConverterFactory (see below).
> Hmmm, a factory seems like an unnecessary addition to the model.

For what reason do you think it's unnecessary? What's the downside of having it? It allows user side control over the converter instantiation (which I think can be crucial) plus might then allow multiple instances of the same converter implementation used for different conversion purposes (global, local).

>> So generally I think a "global-by-default" approach would remove the need to have this special flag at the implementation side of things. We could then have the @Convert annotation look something like this:
> This is just removing the autoApply option at the expense of the
> non-default case (which becomes uglier).

What exactly becomes uglier? Actually you have less annotations to use and do not spread the information about which converter to use over multiple places:

1. If there's a converter registered for the property type use that by default.
2. If the property has a custom convert declared (annotation or XML), use that.
3. If the property has conversion disabled (annotation or XML) do not convert.

Actually the entire @Converter annotation becomes obsolete then if I don't oversee any particular scenario as all the necessary meta information can either be derived from the type or be considered when designing the registration API at the EMF, ConverterFactory.

Cheers,
Ollie

-- 
/**
 * @author Oliver Gierke - Senior Member Technical Staff
 *
 * @param email ogierke_at_vmware.com
 * @param phone +49-351-30929001
 * @param fax   +49-351-418898439
 * @param skype einsdreizehn
 * @see http://www.olivergierke.de
 */