On 17/01/2012 10:33 AM, Oliver Gierke wrote:
> 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?
The Wrapped class would never need to be annotated. However, the
converter class would need to be annotated with @Converter, and if you
wanted it to be applied to every attribute of type Wrapped then
autoApply=true would also need to be set in the @Converter annotation.
>>>> 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).
There's not really much of a win in your example above. The code is
basically going to be a switch statement to support each format (all of
which must be known ahead of time) and I don't really see the value in
having a single super converter that is really just squishing the code
for multiple converters into a single class. If the caller has to know
enough to call a factory to instantiate a converter with the right
format argument then they will know enough to call the correct specific
converter directly. Each converter is very lightweight and does a very
specific conversion, i.e. there is nothing in a converter class over and
above the two (typically short) conversion methods.
>>> 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).
I guess the term "hardcoding" is a subjective thing, but I wouldn't call
a reflective operation on an unknown class hardcoding :-). Yes, the
provider is responsible for creating the converter instances of whatever
converter classes need to be used. I don't see that the additional
complexity of a factory merits the flexibility that you say is
necessary. In fact, I have never had anyone ask for that specific
flexibility.
> Hibernate e.g. allows EventListener instances being registered which is a much more powerful approach.
Sure, every provider has their own design bell or whistle that someone
(or in some cases nobody) uses, but we can't be adding all these to the
spec. We have to draw the line somewhere reasonable, and given that I
have never had anyone need the ability to get their own instances of
these objects I think this is the place to draw it.
>>> 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
If I understand your use case correctly you are saying that they have
some canned converters that are *not* set to autoApply and they now want
to reuse them in a different app but set them to autoApply? It's
possible but I would say less likely. A converter is going to generally
be one of three types:
a) Convert a domain-specific object (e.g. EmployeeStatus). These are
going to be set to autoApply and applied whenever that object type is mapped
b) Convert a known Java type that does not have a corresponding DB type
(e.g. a URL). These will also be set to autoApply since all mapped
objects of that type will typically be converted the same way.
c) Convert an existing Java type to be stored a very specific way in a
specific column (e.g. a Date is stored as a Number in a specific DB
column). This is used by a very limited number of mappings and will not
be set to autoApply.
Pre-existing converters will generally have an autoApply setting that
makes sense, and when reused that setting will still make the most
sense. If, as you suggest, we made all converters autoApply by default
to all attributes of that type then we save the implementer adding a
single attribute in case a) and b), but if ever someone wants the
default to not be autoApply (case c) then you will require that they use
@Convert for every single attribute that *doesn't* need conversion. In
our case c) above that means every Date other than the one that I want
to convert needs a @Convert just to disable the default conversion.
If, in the unlikely case that somebody does want to use a converter that
has an autoApply setting other than the one defined on it then they
would have two choices:
1) Add an overriding entry in the XML file
2) Use the EMF API that I brought up earlier to specify/override the
autoApply option at runtime
> 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).
API bloat for little benefit.
>>> 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:
See my explanation above about how the inverse case becomes ugly because
of all the annotations needed to cause the converter not to be applied.
>
> 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
>