Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attribute converter not called successfully during EntityManager.refresh() call #2096

Open
oramattkosem opened this issue Mar 13, 2024 · 8 comments

Comments

@oramattkosem
Copy link

Describe the bug
Attempting to call EntityManager's refresh method on an entity with an attribute converter leads to a ClassCastException.

jakarta.persistence.PersistenceException: An exception occurred while calling convertToEntityAttribute on converter class test.TestObjectConverter with value ba
	at org.eclipse.persistence.mappings.converters.ConverterClass.convertDataValueToObjectValue(ConverterClass.java:166)
	at org.eclipse.persistence.mappings.foundation.AbstractDirectMapping.getObjectValue(AbstractDirectMapping.java:626)
	at org.eclipse.persistence.mappings.foundation.AbstractDirectMapping.valueFromRow(AbstractDirectMapping.java:1244)
	at org.eclipse.persistence.mappings.foundation.AbstractDirectMapping.buildCloneFromRow(AbstractDirectMapping.java:211)
	at org.eclipse.persistence.internal.descriptors.ObjectBuilder.buildAttributesIntoWorkingCopyClone(ObjectBuilder.java:2111)
	at org.eclipse.persistence.internal.descriptors.ObjectBuilder.buildWorkingCopyCloneFromRow(ObjectBuilder.java:2364)
	at org.eclipse.persistence.internal.descriptors.ObjectBuilder.buildObjectInUnitOfWork(ObjectBuilder.java:958)
	at org.eclipse.persistence.internal.descriptors.ObjectBuilder.buildObjectInternal(ObjectBuilder.java:844)
	at org.eclipse.persistence.internal.descriptors.ObjectBuilder.buildObject(ObjectBuilder.java:799)
	at org.eclipse.persistence.internal.descriptors.ObjectBuilder.buildObject(ObjectBuilder.java:777)
	at org.eclipse.persistence.queries.ObjectLevelReadQuery.buildObject(ObjectLevelReadQuery.java:863)
	at org.eclipse.persistence.queries.ReadObjectQuery.registerResultInUnitOfWork(ReadObjectQuery.java:896)
	at org.eclipse.persistence.queries.ReadObjectQuery.executeObjectLevelReadQuery(ReadObjectQuery.java:570)
	at org.eclipse.persistence.queries.ObjectLevelReadQuery.executeDatabaseQuery(ObjectLevelReadQuery.java:1236)
	at org.eclipse.persistence.queries.DatabaseQuery.execute(DatabaseQuery.java:913)
	at org.eclipse.persistence.queries.ObjectLevelReadQuery.execute(ObjectLevelReadQuery.java:1195)
	at org.eclipse.persistence.queries.ReadObjectQuery.execute(ReadObjectQuery.java:448)
	at org.eclipse.persistence.queries.ObjectLevelReadQuery.executeInUnitOfWork(ObjectLevelReadQuery.java:1283)
	at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.internalExecuteQuery(UnitOfWorkImpl.java:3025)
	at org.eclipse.persistence.internal.sessions.AbstractSession.executeQuery(AbstractSession.java:1841)
	at org.eclipse.persistence.internal.sessions.AbstractSession.executeQuery(AbstractSession.java:1823)
	at org.eclipse.persistence.internal.sessions.AbstractSession.executeQuery(AbstractSession.java:1773)
	at org.eclipse.persistence.internal.jpa.EntityManagerImpl.executeQuery(EntityManagerImpl.java:999)
	at org.eclipse.persistence.internal.jpa.EntityManagerImpl.refresh(EntityManagerImpl.java:1138)
	at org.eclipse.persistence.internal.jpa.EntityManagerImpl.refresh(EntityManagerImpl.java:1029)
	at test.ConverterTest.testConverter(ConverterTest.java:30)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: java.lang.ClassCastException: class java.lang.String cannot be cast to class [C (java.lang.String and [C are in module java.base of loader 'bootstrap')
	at test.TestObjectConverter.convertToEntityAttribute(TestObjectConverter.java:1)
	at org.eclipse.persistence.mappings.converters.ConverterClass.convertDataValueToObjectValue(ConverterClass.java:164)
	... 28 more


To Reproduce
Create an entity like this:

package test;

import jakarta.persistence.*;
import jakarta.validation.constraints.Size;

@Entity
public class TestingEntity {
  private long id;
  private char[] chars;

  @Column(name = "CHARS_STORED_BACKWARDS")
  @Size(max = 256, min = 0)
  @Convert(converter = TestObjectConverter.class)
  public char[] getChars() {
    return chars;
  }

  @Id
  @GeneratedValue(strategy = GenerationType.IDENTITY)
  public long getId() {
    return id;
  }

  public void setChars(char[] argChars) {
    chars = argChars;
  }

  public void setId(long argId) {
    id = argId;
  }
}

And a converter like this:

package test;

import jakarta.persistence.AttributeConverter;
import jakarta.persistence.Converter;

@Converter
public class TestObjectConverter
    implements AttributeConverter<char[], char[]> {

  @Override
  public char[] convertToDatabaseColumn(char[] argNormal) {
    return argNormal != null ? new StringBuilder().append(argNormal).reverse().toString().toCharArray()
        : null;
  }

  @Override
  public char[] convertToEntityAttribute(char[] argReversed) {
    return argReversed != null ? new StringBuilder().append(argReversed).reverse().toString().toCharArray()
        : null;
  }
}

And a PersistenceUnit like this:

<?xml version="1.0" encoding="UTF-8"?>
<persistence xmlns="https://jakarta.ee/xml/ns/persistence" 
             xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
             xsi:schemaLocation="https://jakarta.ee/xml/ns/persistence https://jakarta.ee/xml/ns/persistence/persistence_3_0.xsd" version="3.0">
  <persistence-unit name="test" transaction-type="RESOURCE_LOCAL">
    <provider>org.eclipse.persistence.jpa.PersistenceProvider</provider> 
    <exclude-unlisted-classes>false</exclude-unlisted-classes>
    <properties>
      <property name="eclipselink.logging.level" value="FINE"/>
      <property name="eclipselink.logging.level.sql" value="FINE"/>
      <property name="eclipselink.logging.parameters" value="true"/>
      <property name="eclipselink.ddl-generation" value="drop-and-create-tables" />
      <property name="eclipselink.id-validation" value="NEGATIVE" />
      <property name="eclipselink.cache.shared.default" value="false"/>
      <property name="jakarta.persistence.jdbc.driver" value="org.hsqldb.jdbc.JDBCDriver" />
      <property name="jakarta.persistence.jdbc.url" value="jdbc:hsqldb:mem:tertiary;hsqldb.tx=mvcc" />
      <property name="jakarta.persistence.jdbc.user" value="sa" />
      <property name="jakarta.persistence.jdbc.password" value="" />
    </properties>
  </persistence-unit>
</persistence>

Then, attempt to persist and refresh the entity like this test case:

package test;

import org.junit.jupiter.api.Test;

import jakarta.persistence.*;

public class ConverterTest {
  @Test
  public void testConverter() {
    EntityManagerFactory emf = Persistence.createEntityManagerFactory("test");

    EntityManager em = emf.createEntityManager();

    EntityTransaction tx = em.getTransaction();
    tx.begin();
    TestingEntity entity = new TestingEntity();
    entity.setChars(new char[] {'a', 'b'});
    em.persist(entity);
    em.flush();
    em.refresh(entity);
    tx.commit();
  }
}
  • EclipseLink version 4.0
  • Java/JDK version 21

Expected behavior
The attribute converter should be used for all operations related to the entity.

@oramattkosem oramattkosem changed the title Attribute converter not used during EntityManager.refresh() call Attribute converter not called successfully during EntityManager.refresh() call Mar 13, 2024
@rfelcman
Copy link
Contributor

I think, that issue is in TestObjectConverter. I think, that better implementation is:

@Converter
public class TestObjectConverter
        implements AttributeConverter<char[], String> {

    @Override
    public String convertToDatabaseColumn(char[] argNormal) {
        return argNormal != null ? new StringBuilder().append(argNormal).reverse().toString()
                : null;
    }

    @Override
    public char[] convertToEntityAttribute(String argReversed) {
        return argReversed != null ? new StringBuilder().append(argReversed).reverse().toString().toCharArray()
                : null;
    }
}

Why String instead of char[]? It better suits to JDBC <-> DB text data types mapping.
On the background of em.refresh(entity); is SELECT and java.sql.ResultSet called
https://docs.oracle.com/en/java/javase/21/docs/api/java.sql/java/sql/ResultSet.html
and there no any getXXX() which return char[].
This is reason why public char[] convertToEntityAttribute(char[] argReversed) is not invoked, because it expect
public char[] convertToEntityAttribute(String argReversed) as DB + JDBC produce String.

@oramattkosem
Copy link
Author

oramattkosem commented Mar 15, 2024

The proposed code is a hypothetical example, since I can't share the actual code, but the code in question deals with sensitive information that needs to be flushed from arrays once persisted. Strings cannot be flushed, since they are immutable, so we use char[] in cases like this. The PreparedStatement and ResultSet API methods dealing with Reader and Writer support this use case without a trip through String.

char[] is a standard supported type in JPA and we shouldn't really see a ClassCastException either way. Apart from this refresh() use case, char[] appears to behave correctly everywhere else.

@rfelcman
Copy link
Contributor

Yes char[] is supported in JPA, but I'm speaking about data type fetched from DB by JDBC.
So You mentioned Reader and Writer and which data type is declared in the DB table? In this case maybe converter should be something like public char[] convertToEntityAttribute(java.io.Reader argReversed) {.

@oramattkosem
Copy link
Author

oramattkosem commented Mar 15, 2024

At the database level, the fields are just varchar2 fields. Reader and Writer aren't types listed as supported by JPA, and I don't believe they could reasonably be used in entities either way given their unidirectional and single-use natures.

@rfelcman
Copy link
Contributor

But JPA/EclipseLink depends on JDBC. By default DB varchar2 field is fetched by ResultSet.getString(). This is why I'm mentioning JDBC. DB/JDBC delivers it as a String. In my case Converter input from JPA side is char[] but from DB/JDBC is String. Converter is applied to values delivered by JDBC driver.

@oramattkosem
Copy link
Author

oramattkosem commented Mar 15, 2024

What you're suggesting seems consistent with the Javadoc for converters. Our prior JPA implementation supports this, but we're hoping to switch to EclipseLink.

Given the circumstances, and the state of this in the JPA/JDBC APIs, what is the preferred course of action for storing sensitive information which needs to be cleansed from memory after use in this library? The AttributeConverter API doesn't support the reader/writer paradigm, given that the Y (dbData) parameter must be usable for reading and writing. The only logical choice for this field type seems to be char[], but this seems a bit contradictory to the Note that it is the responsibility of the converter writer to specify the correct dbData type for the corresponding column for use by the JDBC driver: i.e., persistence providers are not expected to do such type conversion. verbiage in the Javadoc.

@rfelcman
Copy link
Contributor

I verified JDBC calls if there is any chance to set/get char[] directly from DB but I don't think, that is possible.
You can try attached code com.oracle.jpa.bugtest.TestBug#bugTestPureJDBCInsert() or com.oracle.jpa.bugtest.TestBug#bugTestPureJDBCSelect.
In case of EclipseLink You can take a look at https://github.com/eclipse-ee4j/eclipselink/pull/2004/files#diff-5f6f66f60c7c0f9a5c9754175e5f1b0a20f01c08961008c848d0c90415145ddf
https://github.com/eclipse-ee4j/eclipselink/blob/master/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/security/SecurableObjectHolder.java as a some idea how we handle passwords in EclipseLink. Or use BLOB, CLOB mapping to use byte[]. But I think, that there is still some moment in every JPA implementation when String is used due a JDBC limitation.
jpa-bug-2096-AttributeConverterEntityManagerRefresh.tar.gz

@oramattkosem
Copy link
Author

oramattkosem commented Mar 19, 2024

You lost me a little bit on that last one. The JPA specification suggests that all basic types other than Id, Temporal, and Enumerated should be supported for conversion. At the JDBC driver level, It is definitely possible to work directly in char to service this use case using the getCharacterStream/setCharacterStream methods. Ignoring the desire to clear the arrays after use, which JPA claims no support for, char[] to String conversions to satisfy the terms of the specification's support for basic types would at least allow the type to be usable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants