SimpleJpaRepository.delete does not respect JPA `@Access` rules [DATAJPA-1565]

[spring-projects-issues]

Leonard Brünings opened DATAJPA-1565 and commented

With the changes of DATAJPA-1535 (c1d99ae) a bug was introduced.

According to JPA rules the position of the annotation defines whether field or property access is used. It can be overridden with the @Access annotation. It seems that org.springframework.data.jpa.repository.support.JpaEntityInformation does not respect that since entityInformation.getId(entity) uses DirectFieldAccessFallbackBeanWrapper which tries the property access first. This can lead to problems if the getter returns a different type than the type of the actual field.

@Entity
public class ExampleEntity {

    @Id
    @Column(columnDefinition = "uuid")
    @GeneratedValue(generator = "uuid2")
    @GenericGenerator(name = "uuid2", strategy = "uuid2")
    private UUID clientId;

    public UUID getId() {
        return clientId;
    }

    public void setClientId(UUID clientId) {
        this.clientId = clientId;
    }

    public String getClientId() {
        return clientId == null ? null : clientId.toString();
    }
}

Fails with (see attached sample)

org.springframework.dao.InvalidDataAccessApiUsageException: Provided id of the wrong type for class com.example.datajpaidaccessbug.db.ExampleEntity. Expected: class java.util.UUID, got class java.lang.String; nested exception is java.lang.IllegalArgumentException: Provided id of the wrong type for class com.example.datajpaidaccessbug.db.ExampleEntity. Expected: class java.util.UUID, got class java.lang.String
 at com.example.datajpaidaccessbug.DatajpaIdAccessBugTests.deletionFails(DatajpaIdAccessBugTests.java:22)

---

Affects: 2.1.9 (Lovelace SR9)

Attachments:

• datajpa-id-access-bug.zip (54.76 kB)

Issue Links:

• DATAJPA-1576 JpaMetamodelEntityInformation.getId(…) uses field access for proxied entities
  ("is duplicated by")

[spring-projects-issues]

Leonard Brünings commented

I hacked around a bit and found a possible solution:

idMetadata.getSimpleIdAttribute().getJavaMember() returns either a Method (property access) or a Field (direct field access), so using that we can use a different bean wrapper.

 

// Line 150
public ID getId(T entity) {
		BeanWrapper entityWrapper = (idMetadata.getSimpleIdAttribute().getJavaMember() instanceof Field) ?
				new DirectFieldAccessBeanWrapper(entity) :
				new DirectFieldAccessFallbackBeanWrapper(entity);
 // ...

With

public class DirectFieldAccessBeanWrapper extends BeanWrapperImpl {

    public DirectFieldAccessBeanWrapper(Object entity) {
        super(entity);
    }

    public DirectFieldAccessBeanWrapper(Class<?> type) {
        super(type);
    }

    @Override
    @Nullable
    public Object getPropertyValue(String propertyName) {
        Field field = findField(getWrappedClass(), propertyName);

        if (field == null) {
            throw new NotReadablePropertyException(getWrappedClass(), propertyName,
                    "Could not find field for property access!");
        }

        makeAccessible(field);
        return getField(field, getWrappedInstance());
    }

    @Override
    public void setPropertyValue(String propertyName, @Nullable Object value) {
        Field field = findField(getWrappedClass(), propertyName);

        if (field == null) {
            throw new NotWritablePropertyException(getWrappedClass(), propertyName,
                    "Could not find field for property access!");
        }

        makeAccessible(field);
        setField(field, getWrappedInstance(), value);
    }
}

It would probably be necessary to change/extend IdentifierDerivingDirectFieldAccessFallbackBeanWrapper too

[spring-projects-issues]

Jens Schauder commented

The interesting question here is: is Spring Data JPA a client to JPA and as such should use getters all the time, strictly speaking not even falling back to fields, or should it be considered an extension of JPA and as such honour the AccessType

[spring-projects-issues]

Leonard Brünings commented

Jens Schauder I would see Spring Data JPA as an extension of JPA

[spring-projects-issues]

shrykull commented

The problem still persists with 2.3.1 - we'd really like to go past version 2.1.8 now...

[spring-projects-issues]

Leonard Brünings commented

Jens Schauder this has been open for over a year now, any chance of progress either way (fix, or rejection)?