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

Types with a @IdClass with a single attribute use the underlying @Id property type within repositories #2330

Closed
payne911 opened this issue Oct 8, 2021 · 3 comments
Assignees
Labels
type: bug A general bug

Comments

@payne911
Copy link

payne911 commented Oct 8, 2021

Context

When my service calls repository.delete(myEntity), I get the following error:

105 | You have provided an instance of an incorrect PK class for this find operation. Class expected : class com.foo.PrimaryKey, Class received : class java.lang.String.

The repository interface extends CrudRepository<MyEntity, PrimaryKey>.

I use @IdClass(PrimaryKey.class) on my @Entity class and have labeled the relevant field with @Id. There is a single field in the PrimaryKey, and it's a String. We do this to keep our codebase consistent so that every @Entity always declares its primary key as a composite key (because many others do use a proper composite key).

Code

  • Repository:
public interface MyEntityCRUDRepository extends CrudRepository<MyEntity, PrimaryKey> { }
public interface MyEntityCustomRepository { /* some custom operations */ }

@Repository
public interface MyEntityRepository extends MyEntityCRUDRepository, MyEntityCustomRepository { }
  • Service:
@Service
@Transactional(rollbackFor = Exception.class)
@RequiredArgsConstructor
public class MyEntityService {
    private final MyEntityRepository repository;

    public void destroy(final PrimaryKey pk) {
//        repository.deleteById(pk); // also returns the error
        MyEntity myEntity = repository.findById(pk)
                .orElseThrow(() -> new IllegalArgumentException(DOES_NOT_EXIST));
        repository.delete(myEntity); // returns the error
    }
}
  • JPA entity:
@Entity
@Table(name = "myentity")
@JsonIgnoreProperties(ignoreUnknown = true)
@Data
@NoArgsConstructor
@AllArgsConstructor
@IdClass(PrimaryKey.class)
public class MyEntity {

    @Id
    @Column(name = "the_id", updatable = false)
    private String theId;

    // other fields which aren't part of the primary key
}
  • Primary key class:
@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
public class PrimaryKey implements Serializable {
    private String theId;
}

Partial analysis (could be wrong)

For some reason, Spring Data JPA internally calls em.find(MyEntity.class, primaryKeyString) instead of building a PrimaryKey instance from that String and using it, hence the error.

The same error happens when I try to call repository.deleteById(correspondingPrimaryKeyInstance) instead.

Framework code

This is where it fails:

T existing = (T) em.find(type, entityInformation.getId(entity));

return (ID) entityWrapper.getPropertyValue(idMetadata.getSimpleIdAttribute().getName());

Somehow, it seems like the (ID) cast has picked up the wrong class to assign to the ID generic (I would assume that it should come from the declared generic of the CrudRepository).

Temporary fix

To temporarily fix the problem, I can remove the @IdClass annotation, and change from:

public interface MyEntityCRUDRepository extends CrudRepository<MyEntity, PrimaryKey> { }

to:

public interface MyEntityCRUDRepository extends CrudRepository<MyEntity, String> { }

but that is not ideal because we would rather have a consistent approach that uses @IdClass for all our @Entity classes throughout the codebase.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 8, 2021
@christophstrobl christophstrobl removed the status: waiting-for-triage An issue we've not yet triaged label Oct 11, 2021
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 11, 2021
@mp911de mp911de changed the title IdClass for single attribute : conflict between CrudRepository and JpaMetamodelEntityInformation Types with a @IdClass with a single attribute use the underlying @Id property type within repositories Oct 12, 2021
@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 12, 2021
@mp911de
Copy link
Member

mp911de commented Oct 12, 2021

Thanks for reporting the issue. That is indeed a bug on our side. The meta-model makes it a bit difficult to identify whether @IdClass is being used but we are able to fix the problem.

mp911de added a commit that referenced this issue Oct 12, 2021
…r repository operations.

We now use the IdClass type for lookups when the entity defines a `@IdClass`. Previously, we uses the type of the defined singular identifier attribute which lead to invalid queries.

Closes #2330
@mp911de mp911de added this to the 2.4.14 (2020.0.14) milestone Oct 12, 2021
mp911de added a commit that referenced this issue Oct 12, 2021
…r repository operations.

We now use the IdClass type for lookups when the entity defines a `@IdClass`. Previously, we uses the type of the defined singular identifier attribute which lead to invalid queries.

Closes #2330
@mp911de
Copy link
Member

mp911de commented Oct 12, 2021

Another workaround is to let your entities implement Persistable.

@payne911
Copy link
Author

payne911 commented Oct 12, 2021

Thank you for the quick fix. :)

I added two comments to your commit. They are probably irrelevant: I'm unaware of Spring's code-style guidelines.

schauder pushed a commit that referenced this issue Jan 21, 2022
…e to obtain IdClass attributes.

We now use IdentifiableType.hasSingleIdAttribute() to detect whether a type should have an IdClass. hasSingleIdAttribute is defined to return false if an IdClass is being used. It could also return false in case multiple identifier attributes are in place (composite Id) which is a bit of a grey area. At least we avoid using Hibernate-specific API.

Original pull request #2412
See #2330
Closes #2391
schauder pushed a commit that referenced this issue Jan 21, 2022
…e to obtain IdClass attributes.

We now use IdentifiableType.hasSingleIdAttribute() to detect whether a type should have an IdClass. hasSingleIdAttribute is defined to return false if an IdClass is being used. It could also return false in case multiple identifier attributes are in place (composite Id) which is a bit of a grey area. At least we avoid using Hibernate-specific API.

Original pull request #2412
See #2330
Closes #2391
schauder pushed a commit that referenced this issue Jan 21, 2022
…e to obtain IdClass attributes.

We now use IdentifiableType.hasSingleIdAttribute() to detect whether a type should have an IdClass. hasSingleIdAttribute is defined to return false if an IdClass is being used. It could also return false in case multiple identifier attributes are in place (composite Id) which is a bit of a grey area. At least we avoid using Hibernate-specific API.

Original pull request #2412
See #2330
Closes #2391
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants