Introduce template method for easier customization of fragments

[MelleD]

I would like to write my own QueryDslExecutor to return a Slice instead of a Page e.g. Slice readAll. However, the QueryDslExecutor is hard coded in the factory.

	@Override
	protected RepositoryComposition.RepositoryFragments getRepositoryFragments(RepositoryMetadata metadata) {

		RepositoryComposition.RepositoryFragments fragments = RepositoryComposition.RepositoryFragments.empty();

		boolean isQueryDslRepository = QUERY_DSL_PRESENT
				&& QuerydslPredicateExecutor.class.isAssignableFrom(metadata.getRepositoryInterface());

		if (isQueryDslRepository) {

			if (metadata.isReactiveRepository()) {
				throw new InvalidDataAccessApiUsageException(
						"Cannot combine Querydsl and reactive repository support in a single interface");
			}

			JpaEntityInformation<?, Serializable> entityInformation = getEntityInformation(metadata.getDomainType());

			Object querydslFragment = getTargetRepositoryViaReflection(QuerydslJpaPredicateExecutor.class, entityInformation,
					entityManager, entityPathResolver, crudMethodMetadataPostProcessor.getCrudMethodMetadata());

			fragments = fragments.append(RepositoryFragment.implemented(querydslFragment));
		}

		return fragments;
	}

In addition, there is no way to override the getTargetRepositoryViaReflection.

	protected final <R> R getTargetRepositoryViaReflection(Class<?> baseClass, Object... constructorArguments) {
		Optional<Constructor<?>> constructor = ReflectionUtils.findConstructor(baseClass, constructorArguments);

		return constructor.map(it -> (R) BeanUtils.instantiateClass(it, constructorArguments))
				.orElseThrow(() -> new IllegalStateException(String.format(
						"No suitable constructor found on %s to match the given arguments: %s. Make sure you implement a constructor taking these",
						baseClass, Arrays.stream(constructorArguments).map(Object::getClass).collect(Collectors.toList()))));
	}

No member variable has a getter, so you have to rely entirely on reflection.

The (ugly) workaround looks like this

   @Override
   protected RepositoryComposition.RepositoryFragments getRepositoryFragments( final RepositoryMetadata metadata ) {

      RepositoryComposition.RepositoryFragments fragments = RepositoryComposition.RepositoryFragments.empty();

      final boolean isQueryDslRepository = QUERY_DSL_PRESENT
            && QuerydslPredicateExecutor.class.isAssignableFrom( metadata.getRepositoryInterface() );

      if ( isQueryDslRepository ) {

         if ( metadata.isReactiveRepository() ) {
            throw new InvalidDataAccessApiUsageException(
                  "Cannot combine Querydsl and reactive repository support in a single interface" );
         }

         final JpaEntityInformation<?, Serializable> entityInformation = getEntityInformation(
               metadata.getDomainType() );

         final Field entityManagerField = ReflectionUtils.findField( getClass(), "entityManager" );
         final Field entityPathResolverField = ReflectionUtils.findField( getClass(), "entityPathResolver" );
         final Field crudMethodMetadataPostProcessorField = ReflectionUtils
               .findField( getClass(), "crudMethodMetadataPostProcessor" );

         final EntityManager entityManager = (EntityManager) ReflectionUtils.getField( entityManagerField, this );
         final EntityPathResolver entityPathResolver = (EntityPathResolver) ReflectionUtils
               .getField( entityPathResolverField, this );
         final CrudMethodMetadataPostProcessor crudMethodMetadataPostProcessor = (CrudMethodMetadataPostProcessor) ReflectionUtils
               .getField( crudMethodMetadataPostProcessorField, this );
         final Object querydslFragment = getTargetRepositoryViaReflection( MySliceableRepositoryImpl.class,
               entityInformation,
               entityManager, entityPathResolver, crudMethodMetadataPostProcessor.getCrudMethodMetadata() );

         fragments = fragments.append( RepositoryFragment.implemented( querydslFragment ) );
      }

      return fragments;
   }

There should be an easier way to provide a custom executor.

[mp911de]

I would like to write my own QueryDslExecutor to return a Slice instead of a Page e.g. Slice readAll. However, the QueryDslExecutor is hard coded in the factory.

You can follow a simpler approach regarding Querydsl by providing a fragment interface that declares the methods as you wish including an implementation. Something along the lines of:

interface MyQuerydslExtension<T> {
    Page<T> findPage(Predicate predicate, Pageable pagable);
}

class MyQuerydslExtensionImpl<T> {
    // …
}

and on the actual repository:

interface PersonRepository implements CrudRepository<Person, String>, QuerydslExecutor<Person>, MyQuerydslExtension<Person> {
}

Spring Data's fragment scanning mechanism detects the MyQuerydslExtension fragment along with its implementation which is way simpler than coming up with a custom repository factory.

[MelleD]

@mp911de thanks for the quick answer. This is exactly what i tried, but then you cannot extend the
QuerydslJpaPredicateExecutor, because of the constructor parameter(s).

This is not working

public class MyQuerydslExtensionImpl<T> extends QuerydslJpaPredicateExecutor<T>
      implements MyQuerydslExtension<T> {

   public MyQuerydslExtensionImpl(
         final JpaEntityInformation<T, ?> entityInformation, final EntityManager entityManager,
         final EntityPathResolver resolver,
         @Nullable final CrudMethodMetadata metadata ) {
      super( entityInformation, entityManager, resolver, metadata );
   }

   @Override
   public Page<T> findAllBy( final Predicate predicate, final Pageable pageable ) {
      final JPQLQuery<T> query = querydsl
            .applyPagination( pageable, createQuery( predicate ).select( path ) );
      final List<T> content = query.fetch();
      return PageableExecutionUtils.getPage( content, pageable, content::size );
   }

[mp911de]

Ah, I see, you want to get hold of the EntityInformation. So overriding getTargetRepositoryViaReflection(…) would be a good fallback assuming you want to subclass QuerydslJpaPredicateExecutor. getRepositoryFragments is working with a lot of package-private types (CrudMethodMetadataPostProcessor) and entityPathResolver isn't exposed, too.

Can you file a ticket in Spring Data Commons so we remove the final keyword from getTargetRepositoryViaReflection?

[MelleD]

I think that would help in my case.
Much is kept private. A few more getter would have been helpful too ;)

[mp911de]

That's by design so we can freely update our implementation without the risk of breaking existing code.

[MelleD]

I am aware of the background. However, it would be very helpful for such adjustments.
One could point out in the Javadoc that it is for internal purposes and can break cusotomizing. Better to break the code with an update, then you know what to do. But philosophy ask :)

[mp911de]

After careful reconsideration, we decided to revisit this topic. A lot of constructor arguments are hidden within JpaRepositoryFactory and are difficult to obtain. We want to introduce a template method getRepositoryFragments(RepositoryMetadata, JpaEntityInformation<T, ?>, EntityManager, EntityPathResolver, CrudMethodMetadata) and move QuerydslJpaPredicateExecutor creation into that method so you can easier get hold of the actual method parameters.

[MelleD]

@mp911de then maybe my PR is obsolet spring-projects/spring-data-commons#2360

[mp911de]

Yeah, I think so. Also, the method name is a bit misleading so we want to rename getTargetRepositoryViaReflection to a better name (deprecate the old one and introduce a replacement method), see spring-projects/spring-data-commons/issues/2361.

In any case, thanks for your contribution.

[MelleD]

With pleasure.
Thank you for the quick processing!

[MelleD]

Hello @mp911de are here any news?
Does this extension already exists (i cannot find the method)?

If not, could we remove the final from instantiateClass?