Expose generic repository types through `RepositoryMetadata`

[alienisty]

If I have a class Id and a domain class Domain and I defined a repository as:

public interface StringDomainRepository extends CrudRepository<Id<String>, Domain<String>> {}

Then the RepositoryMetadata getIdType() and getDomainType() methods will return the erased types (Id and Domain) even though the information would be there and could be returned as Type rather than Class<?>.

I propose to change the interface as:

/**
 * Metadata for repository interfaces.
 *
 * @author Oliver Gierke
 * @author Mark Paluch
 */
public interface RepositoryMetadata {

	/**
	 * Returns the id class the given class is declared for.
	 *
	 * @return the id class of the entity managed by the repository.
	 */
	TypeInformation<?> getIdType();

	/**
	 * Returns the domain class the repository is declared for.
	 *
	 * @return the domain class the repository is handling.
	 */
	TypeInformation<?> getDomainType();

	/**
	 * Returns the repository interface.
	 *
	 * @return
	 */
	Class<?> getRepositoryInterface();

	/**
	 * Returns the type {@link Method} return type as it is declared in the repository. Considers suspended methods and
	 * does not unwrap component types but leaves those for further inspection.
	 *
	 * @param method
	 * @return
	 * @since 2.4
	 */
	TypeInformation<?> getReturnType(Method method);

	/**
	 * Returns the domain class returned by the given {@link Method}. In contrast to {@link #getReturnType(Method)}, this
	 * method extracts the type from {@link Collection}s and {@link org.springframework.data.domain.Page} as well.
	 *
	 * @param method
	 * @return
	 * @see #getReturnType(Method)
	 */
	Class<?> getReturnedDomainClass(Method method);

	/**
	 * Returns {@link CrudMethods} meta information for the repository.
	 *
	 * @return
	 */
	CrudMethods getCrudMethods();

	/**
	 * Returns whether the repository is a paging one.
	 *
	 * @return
	 */
	boolean isPagingRepository();

	/**
	 * Returns the set of types the repository shall be discoverable for when trying to look up a repository by domain
	 * type.
	 *
	 * @see Repositories#getRepositoryFor(Class)
	 * @return the set of types the repository shall be discoverable for when trying to look up a repository by domain
	 *         type, must not be {@literal null}.
	 * @since 1.11
	 */
	Set<Class<?> getAlternativeDomainTypes();

	/**
	 * Returns whether the repository is a reactive one, i.e. if it uses reactive types in one of its methods.
	 *
	 * @return
	 * @since 2.0
	 */
	boolean isReactiveRepository();
}

This way lower level components, like for example GenericConverters, would be able to have the full type information.

[alienisty]

I've implemented the change but I didn't create the pull request yet because API changes need discussion first.

[mp911de]

We generally do not assume parametrized types as all implementations work on the level of Class to fetch objects. Can you elaborate on your use case? We would like to learn about your arrangement.

[alienisty]

In my specific case, I have a repository for which the id type is itself generics. Now, when spring-data builds the RepositoryMetadata for a repository interface, it has all the information as TypeInformation instances, but, with the current API, such information is dropped exposing just the raw types for the id and domain type.
Such loss of information causes a problem with convertions because spring data ends up using

<T> T ConversionService.convert(@Nullable Object source, Class<T> targetType);

rather than

Object ConversionService.convert(@Nullable Object source, @Nullable TypeDescriptor sourceType, TypeDescriptor targetType);

More specifically, in DomainClassConverter, both ToEntityConverter and ToIdConverter invoke the former style of conversion method, which lacks generics information, and, in our case, that is needed to properly convert the id instance.

Another place where spring-data uses the former style of conversion is in ReflectionRepositoryInvoker.

Note that an instance value's type does not always have all generics parameter reified, whereas the signature of the id and domain type extrated from the repository interface would have them in all practical cases (non abstract that is).

In the end, the proposed change is augmenting the API and not reducing it, in fact, calling getType() on TypeInformation would give us back the same class returned by the current implementation.

I hope this clarifies my position, if not, please let me know and I'll try to rephrase.

If it helps I could attach the patch for the change I had in mind, which I think could be futher reduced by using default methods.

[mp911de]

Thanks for the additional details. Considering DomainClassConverter and other components helps to clarify the intended usage. Feel free to submit a pull request. Please also note that annotation-based repository declarations (@RepositoryDefinition(domainClass=Domain.class)) isn't capable of considering generics.

[alienisty]

Yes, that is a good point, I noticed the @RepositoryDefinition while parsing the codebase. I never actually used it so I will need to look into it better, but I guess you would not be using that if generics are involved, at least I would not.
I'll review and cleanup my branch and will submit the PR.

Quick question on the build. I'm not sure what am I doing wrong, but if I don't add the dependency to jakarta.xml.bind-api in the pom, the build fails. Is there another way you get that dependency? I didn't seem to find any special mention about it in the documentation.

[mp911de]

Please use either Java 8 to build the project or run the build with the java11 Maven profile (-Pjava11)