[rzo1]

Reported on the dev list by @ilgrosso, testing Syncope master against 4.2.0-SNAPSHOT: @Convert on a List attribute whose converter is declared AttributeConverter<List<X>, Y>. Such a converter converts the attribute as a whole, so the attribute is basic and belongs in one column. OpenJPA instead found no persistence strategy for it, left it out of the mapping, and lost its contents without raising anything — only one warning among several hundred of the same kind in a full run.

The element handler for collections was already there; the attribute never reached it. Three places decide this, and all three only ever asked the type code:

• PersistenceMetaDataDefaults.getPersistenceStrategy() now treats a converter of the attribute's own type as a strategy, i.e. BASIC, as the last resort before giving up on the field.
• parseBasic() no longer serializes or rejects a collection in that case, since the converter maps the value.
• MappingRepository.defaultTypeStrategy() and FieldMetaData.isValueConverted() admit a collection or map whose converter takes the whole value, which gives it ConverterValueHandler and a single column.

The discriminator is the converter's own declaration, read from AttributeConverter<X,Y>: X assignable from the attribute type means the value is converted as a whole, X being the element type leaves the existing element-wise path untouched. FieldMetaData.resolveConverterTypeArgument() is the generalisation of the resolver that already read Y.

Out of scope: element-wise conversion of a collection of a non-basic type, which the specification defines for @Convert on an @ElementCollection and which OpenJPA does not support yet — its enhancer demands metadata for the element type. Worth its own issue.

Both tests fail without the production change, verified by reverting it: the converted attribute must be mapped, and expected:<2> but was:<0> for the round trip. Conversion, enhance, meta, simple and embed packages run 621/29/39 on Derby and on H2, against 619/29/39 on master at the same base with an identical list of failing classes, so the two new tests are the only difference. The unit tests of the three changed modules are green: kernel 145, jdbc 75, persistence 34.

[ilgrosso]

LGTM thank you!

[rzo1]

@ilgrosso thanks for testing. The converter is fine and matches yours. The difference is in META-INF/spring-orm.xml, which maps plainAttrs again as

<basic name="plainAttrs">
  <column column-definition="jsonb"/>
</basic>

The first fix only handled annotations. When OpenJPA read a <column> for a collection field from a mapping file, it put the column on the list elements instead of on the field. A list converted by an AttributeConverter<List<X>, String> has no element columns, so mapping failed with the error you saw. A <column> inside <basic> is now used for the field itself. TestConvertedListXml covers this with a mapping file like the one in Syncope.

There is one more change needed on your side. The fields are declared private final List<PlainAttr> plainAttrs. JPA does not allow persistent fields to be final, and the OpenJPA enhancer skips them. With annotations only, the field is not persisted and no error is raised. With the <basic> entry the field is persistent at runtime but missing from the enhanced class, so the first persist() fails in pcProvideField.

Could you remove final from plainAttrs in JPAAnyObject, JPAGroup, JPAUser, JPALinkedAccount and JPARealm, rebuild against the current head of OPENJPA-3017, and try again?

[ilgrosso]

Thank you @rzo1 : once pulled, rebuilt and removed final as per your suggestion, things are getting better.

At present I am getting

org.apache.openjpa.lib.jdbc.ReportingSQLException: ERROR: type modifier is not allowed for type "jsonb"
  Posizione: 292 {stmnt 853321803 
CREATE TABLE AnyObject (id VARCHAR(36) NOT NULL, creationContext VARCHAR(255), creationDate TIMESTAMP WITH TIME ZONE, 
        creator VARCHAR(255), lastChangeContext VARCHAR(255), lastChangeDate TIMESTAMP WITH TIME ZONE, lastModifier 
        VARCHAR(255), status VARCHAR(255), name VARCHAR(255), plainAttrs jsonb(255), GMANAGER_ID VARCHAR(36), REALM_ID 
        VARCHAR(36), UMANAGER_ID VARCHAR(36), TYPE_ID VARCHAR(255), PRIMARY KEY (id), CONSTRAINT U_NYOBJCT_NAME UNIQUE 
        (name, TYPE_ID)) } 

so it looks like something attempts to set jsonb(255) as column type instead of jsonb as indicated by spring-orm.xml?

[rzo1]

@ilgrosso thanks, good to see it gets further now. The jsonb(255) is a separate problem. OpenJPA appends the column length to a column definition unless the dictionary knows the type has a fixed size, and JSONB is not in that list for PostgreSQL (neither is JSON for MySQL and MariaDB). Syncope 3.x did not hit this because its mapping also had <lob/>, which removes the length.

I opened OPENJPA-3018 to fix it in the dictionaries. Until then you can work around it with:

openjpa.jdbc.DBDictionary=postgres(FixedSizeTypeNames=JSONB)

[ilgrosso]

Great progress, thank you @rzo1!
I've locally cherry-picked the commit from #221 into this branch and built, then run again the full JPA tests in Syncope.

We moved from

Tests run: 352, Failures: 0, Errors: 352, Skipped: 0

to

Tests run: 352, Failures: 15, Errors: 37, Skipped: 0

I will have a look at the failures and report.

[ilgrosso]

It looks like one reason of a few test failures is that the following test code:

User rossini = userDAO.findByUsername("rossini").orElseThrow();

PlainAttr loginDate = rossini.getPlainAttr("loginDate").orElseThrow();
loginDate.getValues().getFirst().setDateValue(FormatUtils.parseDate(LOGIN_DATE_VALUE, "yyyy-MM-dd"));

entityManager.merge(rossini);

does not generate any UPDATE statement while this code:

User rossini = userDAO.findByUsername("rossini").orElseThrow();

PlainAttr loginDate = rossini.getPlainAttr("loginDate").orElseThrow();
rossini.remove(loginDate);
loginDate.getValues().getFirst().setDateValue(FormatUtils.parseDate(LOGIN_DATE_VALUE, "yyyy-MM-dd"));
rossini.add(loginDate);

entityManager.merge(rossini);

does.

In the first snippet, the converted List<PlainAttr> is read (getPlainAttr() does getPlainAttrs().filter(...).findFirst() internally) and one of its element is modified.

In the second snippet, the converted List<PlainAttr> is actually modified.

[rzo1]

@ilgrosso thanks, that is a bug: the spec requires a change made in place to a mutable value to be written. Tracked in OPENJPA-3019.

[rzo1]

@ilgrosso regarding the BYTEA column for @Lob @Convert(converter = ProvisionListConverter.class) List<Provision> provisions from your mail: with @Lob, OpenJPA chose CLOB or BLOB from the attribute type (List, so BLOB) instead of the converter database type (String, so CLOB). This is OPENJPA-3020, fixed in #222, which is based on this branch. The same applies to MySQL, MariaDB and Oracle, where you get a BLOB instead of TEXT or CLOB.

Until it is merged, you can drop @Lob and declare the column type, e.g. @Column(columnDefinition = "TEXT") on PostgreSQL.

[ilgrosso]

Great job @rzo1 !

With the combined changes from this PR, #221 #222 and #223 I am seeing

[ERROR] Tests run: 352, Failures: 9, Errors: 5, Skipped: 0