[clarification]: non-repository interfaces do not have primary entity types and so queries must explicit the entity type

[gavinking]

Specification

@query on non-repository interfaces

I need clarification on ...

The TCK has this hidden treasure:

public interface DepartmentQueries<T> {

    @Query("where department = :dept order by name")
    List<T> findByDepartment(@Param("dept") String department);

    @Query("select count(*) where department = :dept")
    long countByDepartment(@Param("dept") String department);

    @Query("where name like :pattern order by name")
    List<T> searchByName(@Param("pattern") String pattern);
}

@Repository
public interface ProjectRegistry extends DataRepository<Project, Long>, DepartmentQueries<Project> {

    @Save
    void save(Project project);

    @Query("where budget > :min order by name")
    List<Project> fundedAbove(@Param("min") double min);
}

Let us now consider this gem of a method declaration in all its radiant glory:

    @Query("where department = :dept order by name")
    List<T> findByDepartment(@Param("dept") String department);

From the signature, we infer the query:

from T t where t.department = :dept order by t.name

But the generic type T cannot possibly have fields named department and name.

Oh, but a concrete argument to T might happen to be the class Project which just happens by complete coincidence to have such fields, so it's OK, right?

No. No, it's not okay. That's not how types work.

We should clarify in the spec that this is not supported, and we should delete the code from the TCK.

Additional information

UPDATE: Sorry! The TCK does not contain the code above! See comments below.

[gavinking]

Well, in fact the spec already disallows this code:

A repository interface may inherit methods from a superinterface.
A superinterface of a repository interface must either:

• be one of the built-in generic repository supertypes defined by this specification, DataRepository, BasicRepository, or CrudRepository, or
• be a non-generic toplevel interface with no type parameters, whose abstract methods likewise declare no type parameters, and which does not itself directly or indirectly inherit any generic interface or any interface whose abstract methods declare type parameters.

So we did in fact anticipate this possibility and rejected it.

So this is merely a bug in the TCK.

[marko-bekhta]

Apologies for the confusion caused... it was me looking at

public interface AsciiCharacters extends DataRepository<AsciiCharacter, Long>, IdOperations

and assuming that DataRepository was a custom interface with queries. 😕 Which resulted in a test that Gavin has in the description.

[gavinking]

OK, we got our wires a bit crossed over here. The actual test that @marko-bekhta ran into isn't as wrong as the one described above. But I still don't love this:

public interface IdOperations {
    long countByIdBetween(long minimum, long maximum);

    boolean existsById(long id);

    @Query("SELECT id WHERE id >= :inclusiveMin ORDER BY id ASC")
    List<Long> withIdEqualOrAbove(long inclusiveMin, Limit limit);
}

My issue with this is that the interface has, in isolation, no well-defined primary entity type, and yet it assumes things: every one of those methods assumes that the entity which it knows nothing about has an identifier of type Long.

This means that the interface cannot be type checked in isolation.

As a practical matter, my annotation processor simply can't type check this code because it only has local error messages to work with.

I therefore think we should ban interfaces with no well-defined primary entity from declaring operations which depend on a primary entity type.

[njr-11]

I'm glad to hear DepartmentQueries<T> isn't in the TCK. I've been trying to stay aware of coverage that gets added and I didn't remember that one. I do vaguely remember IdOperations being related to a functionality request that someone had made during Data 1.0, but I don't remember who. It's likely this was pretty early on in 1.0 and we added the more restrictive language later, likely making what was requested less useful to them.

As far as the TCK goes, we could easily remove IdOperations and copy its 3 methods directly into the two repositories that were inheriting the methods from it.

[gavinking]

As far as the TCK goes, we could easily remove IdOperations and copy its 3 methods directly into the two repositories that were inheriting the methods from it.

Yeah, done in the linked PR. But we might want to add some new test to show inheritance from a superinterface, which is definitely still allowed, just one which has operations with well-defined types.