serializable types as basic types

[gavinking]

Since 1.0, the Persistence spec has said that any un-annotated field is basic by default, and also provided a fall-through to Java serialization for unrecognized basic types.

This is a misfeature.

• Storing serialized Java objects in the database is almost always a rubbish thing to do.
• The fall-through prevents the provider from providing meaningful error reports for unrecognized types, or when mapping annotations are missing.
• In case a user really wants this, it's quite straightforward to provide an AttributeConverter.

So I think we should at least deprecate this, and perhaps even consider having some way to just turn this "feature" off.

Thoughts?

[AleksNo]

And it is a long term goal to remove serialization from Java:
https://adtmag.com/articles/2018/05/30/java-serialization.aspx

If it happens then the ORM vendors would need to develop and maintain a seriallization framework by themself. So, turning it off is surely a good idea.

[gavinking]

So if we did this, exactly how would we go about it?

I guess I would kind of like to give providers a way to complain when the application program makes use of a deprecated feature like:

1. java.sql date/time types
2. serializable fields

while still supporting backward compatibility for legacy applications.

I wonder if it might be OK to involve the version of the persistence.xml XSD in this, so that:

• for older persistence schemas, these features are accepted silently, but
• for the version 4.0 persistence.xml schema, the provider has the option of rejecting or complaining about use of these features unless some backward compatibility setting is enabled.

Thoughts?

[sebersole]

I'd just say that support for non-annotated attributes with types beyond the explicitly supported basic types (already defined) is vendor specific. In other words, strictly speaking, the spec only requires support for attributes which are -

1. explicitly annotated, or
2. in that discrete list of basic types.

I know you generally hate switches, and generally rightfully so, but this feels like a place we should absolutely have one. Using the JPA version would be an interesting option here and I'm +1 for that.

[gavinking]

@sebersole The problem is that in the definition of a "basic type", the spec says this:

any other type which implements java.io.Serializable.

though it never defines how such types are supposed to be persisted.

[sebersole]

Well, clearly, remove that entry as well ;)

I thought that was given in relation to an issue discussing dropping support for serializable

[AleksNo]

I am also curious how often this "feature" is used, that un-annotated types fall back to Java serialization if these types implement java.io.serializable. If nobody uses this feature then a deprecation and removal should be a no-brainer. For me, it seems to be a very rare combination. But i can also imagine, that it can become very complex to migrate away from it.

[gavinking]

If nobody uses this feature

I'm sure some people use it, if only because it has been there for a very long time.

But i can also imagine, that it can become very complex to migrate away from it.

I agree, we need to have some sort of backward compatibility story. However, I think I am prepared to argue that because this feature was never properly specified (the spec doesn't say anything at all about how a serializable field should be persisted), and since everyone agrees it was a misfeature, that the requirements for backward compatibility here should be considered much weaker than they are for more fully specified features.

[gavinking]

As an extra data point, apparently the TCK does not exercise this feature (at least according to Sol).

The spec requires arbitrary Serializable types as basic attributes (ch02-entities.adoc), and assertion SPEC:528 claims this coverage (spec-assertions-3_2.html). But its tests only exercise array types such as byte[]/char[] (FieldTypeClient.java) and Byte[]/Character[] (PropertyTypePropertyTypeClient.java).
There is no test with an arbitrary user-defined Serializable basic field persisted, cleared, and reloaded. The array tests also inspect the same managed instance after flush(). So this appears to be a genuine TCK coverage gap.

So that's more evidence that this feature was never actually portable.

[gavinking]

Thinking through this some more, it really hits me just how under specified this thing is. An implementation might persist to a UDT, to JSON, to XML, to binary, or something else and still be compliant. There's just no reasonable expectation of portability here. Especially given that the TCK does not cover it.

So frankly I think it would be OK to just say the code is unportable and let implementations sort out backward compatibility questions in a way which makes sense for their users.

[AleksNo]

OK, i used Grok to find files or projects, which would be affected by this feature. I asked Grok:
"Find Java files in Java projects, which contain following properties: JPA or Jakarta Persistence entities, with fields which have following properties: they are not annotated, they implement java.io.Serializable and they are not the following types: ( java.lang.String, java.math.BigInteger, java.math.BigDecimal, java.util.UUID, java.util.Date, java.util.Calendar, java.sql.Date, java.sql.Time, java.sql.Timestamp, byte[], Byte[], char[], Character[], java.time.LocalDate, java.time.LocalTime, java.time.LocalDateTime, java.time.OffsetTime, java.time.OffsetDateTime, java.lang.Integer, java.lang.Byte, java.lang.Short, java.lang.Long, java.lang.Float,java.lang.Double, java.lang.Boolean, java.lang.Character). List them all in a simple list."

After it found one Java file in the Hibernate project, i asked Grok: "are there more files? if so list them all in a simple list"

And it found:
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/bitset/BitSetImplicitTests.java
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/CurrencyMappingTests.java
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/LocaleMappingTests.java
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/ClassMappingTests.java
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/TimeZoneMappingTests.java

Then i asked the same in german. And it found an addirional project with this file:
https://github.com/otrosien/jpa-monetaryamount-example/blob/master/src/main/java/com/example/Product.java

So it seems, this feature is not widely used.

[gavinking]

Should be: "they are not the following types, and they are not affected by an auto-apply converter".

[AleksNo]

OK, i did it. It finds these URLs:
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/bitset/BitSetImplicitTests.java
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/CurrencyMappingTests.java
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/LocaleMappingTests.java
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/ClassMappingTests.java
https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/TimeZoneMappingTests.java

I also added java.time.Instant and java.time.Year to the exclusion list because these types will be added to Jakarta Persistence 4.0 as supported basic types.

Here is the prompt:
"Find Java files in Java projects, which contain following properties: JPA or Jakarta Persistence entities, with fields which have following properties: they are not annotated, they implement java.io.Serializable and they are not the following types: ( java.lang.String, java.math.BigInteger, java.math.BigDecimal, java.util.UUID, java.util.Date, java.util.Calendar, java.sql.Date, java.sql.Time, java.sql.Timestamp, byte[], Byte[], char[], Character[], java.time.LocalDate, java.time.LocalTime, java.time.LocalDateTime, java.time.OffsetTime, java.time.OffsetDateTime, java.time.Instant, java.time.Year, java.lang.Integer, java.lang.Byte, java.lang.Short, java.lang.Long, java.lang.Float,java.lang.Double, java.lang.Boolean, java.lang.Character), and they are not affected by an auto-apply converter. Do not search on the local computer. List them all in a simple list of URLs."