Spring Data Repositories: Validate ID type

[mp911de]

It would make sense to add validation for Spring Data repository definitions, specifically to check that the declared repository ID type is assignable to (compatible with) the domain type. The validation should affect the repository definition and should be triggered by the presence of a repository declaration.

Consider the following declarations:

class Customer {
    
    @Id String id;
}

interface CustomerRepository extends Repository<Customer, Long>{}

@RepositoryDefinition(domainClass = Customer.class, idClass = Long.class)
interface CustomerRepository {}

The ID type in the entity is String while the repositories declare Long. This code indicates a potential bug.
The validation should check whether the repository ID type can be assigned from the entity ID type or vice versa. Entities may declare a primitive type so the validation needs to consider primitive wrapper types, too.

Valid cases

class Customer {
    
    @Id String id;
}

interface CustomerRepository extends Repository<Customer, CharSequence>{}

@RepositoryDefinition(domainClass = Customer.class, idClass = CharSequence.class)
interface CustomerRepository {}

class Customer {
    
    @Id CharSequence id;
}

interface CustomerRepository extends Repository<Customer, String>{}

@RepositoryDefinition(domainClass = Customer.class, idClass = String.class)
interface CustomerRepository {}

class Customer {
    
    @Id int id;
}

interface CustomerRepository extends Repository<Customer, Integer>{}

@RepositoryDefinition(domainClass = Customer.class, idClass = Integer.class)
interface CustomerRepository {}

Invalid cases

class Customer {
    
    @Id String id;
}

interface CustomerRepository extends Repository<Customer, Long>{}

@RepositoryDefinition(domainClass = Customer.class, idClass = Long.class)
interface CustomerRepository {}

class Customer {
    
    @Id String id;
}

interface CustomerRepository extends Repository<Customer, Number>{}

@RepositoryDefinition(domainClass = Customer.class, idClass = Number.class)
interface CustomerRepository {}

class Customer {
    
    @Id Integer id;
}

interface CustomerRepository extends Repository<Customer, Double>{}

@RepositoryDefinition(domainClass = Customer.class, idClass = Double.class)
interface CustomerRepository {}

Advanced cases

Sometimes, entities do not declare an Id type so the validation is not possible. In such a case, the validation should be ignored.

class Customer {
    
    String name;
}

interface CustomerRepository extends Repository<Customer, Integer>{}

@RepositoryDefinition(domainClass = Customer.class, idClass = Integer.class)
interface CustomerRepository {}

Types annotated with @NoRepositoryBean indicate framework types and should not be validated. Their subtypes are (unless annotated with @NoRepositoryBean) should be validated.

@NoRepositoryBean
interface CustomerRepository<T extends MyType, ID extends Serializable> Repository<T, ID>{}

@NoRepositoryBean
interface CrudRepository<T, ID> extends Repository<T, ID> 

We've seen various levels of generics usage. While this is not the most common case, it would make sense to consider these cases for validation.

class Customer {
    
    @Id String id;
}

interface CustomerRepository<T extends Customer, ID extends Long> extends Repository<T, ID>{}


@NoRepositoryBean
interface MyIntermediateRepository<T extends Customer, ID extends Number> extends Repository<T, ID>{}

interface MyConcreteRepository extends MyIntermediateRepository<Customer, Long>{}


@NoRepositoryBean
interface MyOtherIntermediateRepository1<T extends Customer> extends Repository<T, Long>{}

interface MyOtherConcreteRepository1 extends MyOtherIntermediateRepository1<Customer>{}


@NoRepositoryBean
interface MyOtherIntermediateRepository2<ID extends Number> extends Repository<Customer, ID>{}

interface MyOtherConcreteRepository2 extends MyOtherIntermediateRepository2<Long>{}

[martinlippert]

This is great @mp911de, thanks for the comprehensive list of examples, much appreciated.

[BoykoAlex]

@mp911de The @Id annotation... I suppose there must be only one member of the class at most marked with this annotation, correct? If yes, then perhaps this is another validation we could create... If something is annotated with @Id in the superclass then i suppose the id member has been picked correct? Does it make sense to have @Id on an interface method? I suppose not because it doesn't have @Inherited...

[BoykoAlex]

FYI: I'm thinking just to provide validation without the quickfix... The quick fix might involve not just changing type declaration annotation or extends/implements parameter type but also likely change signatures of implemented methods etc.

[mp911de]

Basically, yes, there must be at most a single @Id annotation. For e.g. MongoDB, we imply the identifier also via name, if a property is named id because of the MongoDB primary key spec.

In Cassandra, we support composite primary keys, as embedded properties (https://github.com/spring-projects/spring-data-cassandra/blob/b793c94eed89aa602bc9b55a2cb057b9844c3949/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/convert/SchemaFactoryUnitTests.java#L184-L188) or primary key class (https://github.com/spring-projects/spring-data-cassandra/blob/b793c94eed89aa602bc9b55a2cb057b9844c3949/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/convert/SchemaFactoryUnitTests.java#L157C1-L180)

[BoykoAlex]

@mp911de My understanding is for the Cassandra case that if in the domain class i find a member of type X where X is annotated with @org.springframework.data.cassandra.core.mapping.PrimaryKeyClass then i can assume that type X is the ID class, correct? I suppose that is if explicit @Id is missing, right? Or would it take precedence over @Id?
When it comes to MongoDB case with a property named id i suppose i should have some spring MongoDB lib on the classpath in this case? If yes, what are the group id and artifact id of the lib?

I'm finishing off rewrite recipe for the explicit @Id case. Will put a link here to rewrite-spring project PR for you to have a look. Most important part to review would be the tests to ensure we cover the important cases. (All cases from then description will be in the tests and a bit more)

[BoykoAlex]

@mp911de Here is the rewrite-spring PR: openrewrite/rewrite-spring#355 Please have a look at the test coverage. If there are more cases you might think of feel free to paste code snippets here or in the PR directly. Note that for now domain id is found by considering only @Id marked members. I think we can improve it with MongoDB implicit id as well as Cassandra primary key class annotation once i have a bit better understanding of these 2 cases.

Once the PR is in I'll add a bit of "glue" code in STS to mark the related code with an error.

[BoykoAlex]

This is likely to be affected by the solution to #987

[BoykoAlex]

@mp911de i have added support for MongoDB id field name. The id field will be taken into account while looking for domain id type when spring-data-mongodb-<version> JAR is detected on the classpath. Seems like in the Cassandra case one would still need to annotate the domain id with Id or its "sub-annotations" (i.e. PrimaryKey).

[BoykoAlex]

Should be fixed with 73b6c6b