SimpleJpaRepository.deleteAllByIdInBatch accepts Iterable but JPA requires Collection

[onlyonce]

SpringBoot 2.5.1

SimpleJpaRepository.deleteAllByIdInBatch(Iterable<ID> ids) accepts Iterable, but in deep QueryParameterBindingValidator.validate checking against for Collection.

	public <P> void validate(Type paramType, Object bind, TemporalType temporalType) {
		if ( bind == null || paramType == null ) {
			// nothing we can check
			return;
		}
		final Class parameterType = paramType.getReturnedClass();
		if ( parameterType == null ) {
			// nothing we can check
			return;
		}

		if ( Collection.class.isInstance( bind ) && !Collection.class.isAssignableFrom( parameterType ) ) {
			// we have a collection passed in where we are expecting a non-collection.
			// 		NOTE : this can happen in Hibernate's notion of "parameter list" binding
			// 		NOTE2 : the case of a collection value and an expected collection (if that can even happen)
			//			will fall through to the main check.
			validateCollectionValuedParameterBinding( parameterType, (Collection) bind, temporalType );
		}
		else if ( bind.getClass().isArray() ) {
			validateArrayValuedParameterBinding( parameterType, bind, temporalType );
		}
		else {
			if ( !isValidBindValue( parameterType, bind, temporalType ) ) {
				throw new IllegalArgumentException(
						String.format(
								"Parameter value [%s] did not match expected type [%s (%s)]",
								bind,
								parameterType.getName(),
								extractName( temporalType )
						)
				);
			}
		}
	}

If calling SimpleJpaRepository.deleteAllByIdInBatch with Iterable but this class not implement Collection we got exception.

Caused by: java.lang.IllegalArgumentException: Parameter value [xxx] did not match expected type [java.util.UUID (n/a)]
        at org.hibernate.query.spi.QueryParameterBindingValidator.validate(QueryParameterBindingValidator.java:54)
        at org.hibernate.query.spi.QueryParameterBindingValidator.validate(QueryParameterBindingValidator.java:27)
        at org.hibernate.query.internal.QueryParameterBindingImpl.validate(QueryParameterBindingImpl.java:90)
        at org.hibernate.query.internal.QueryParameterBindingImpl.setBindValue(QueryParameterBindingImpl.java:55)
        at org.hibernate.query.internal.AbstractProducedQuery.setParameter(AbstractProducedQuery.java:494)
        at org.hibernate.query.internal.AbstractProducedQuery.setParameter(AbstractProducedQuery.java:115)
        at org.springframework.data.jpa.repository.support.SimpleJpaRepository.deleteAllByIdInBatch(SimpleJpaRepository.java:229)

e.g.:

/**
 * Add {@link Iterable} ability to {@link Stream}.
 */
@RequiredArgsConstructor
public class IterableStream<T> implements Iterable<T> {

  private final Stream<T> stream;


  @Override
  public Iterator<T> iterator() {
    return stream.iterator();
  }
}

[gregturn]

I reproduced your issue by writing a tiny bit of test code...

@Test // 2242
void deleteAllByIdInBatchViaIterable() {

	SampleEntity one = new SampleEntity("one", "eins");
	SampleEntity two = new SampleEntity("two", "zwei");
	SampleEntity three = new SampleEntity("three", "drei");
	repository.saveAll(Arrays.asList(one, two, three));
	repository.flush();

	Iterable<SampleEntityPK> iterableIds = new IterableStream<>(Stream.of(new SampleEntityPK("one", "eins"), new SampleEntityPK("three", "drei")));
	repository
			.deleteAllByIdInBatch(iterableIds);
	assertThat(repository.findAll()).containsExactly(two);
}

static class IterableStream<T> implements Iterable<T> {

	private final Stream<T> stream;

	IterableStream(Stream<T> stream) {
		this.stream = stream;
	}

	@Override
	public Iterator<T> iterator() {
		return this.stream.iterator();
	}
}

And indeed, it DOES fail here. But the context is a bit more subtle. The place it's failing is inside Hibernate itself. Hibernate clearly expects to receive a Collection, not an Iterable.

However, Spring Data JPA doesn't go directly to Hibernate. Instead, it's leaning on JpaRepository, an interface that extends from Spring Data Commons. And Spring Data Commons has a long-standing history of leveraging the widest collection type available Iterable, when it comes to inputs.

It's true that JpaRepository, the JPA-specific extension of CrudRepository has List for return types but it holds to accepting Iterable for inputs. This is merely a combination of Postel's Law ("be conservative in what you send, be liberal in what you accept") lining up with what we know is returned by Hibernate (List).

So this puts us in a gray area. I can see justification that if Hibernate itself is looking for a Collection, we might need to alter this input. But I'd like to check the other areas where we have inputs on JpaRepository that are then fed to Hibernate and see if Hibernate is consistently looking for a Collection in those areas as well.

Altering these signatures would be a breaking change, and so we need to weigh it properly, which is why I'd also like feedback from @schauder and possibly @odrotbohm.

[schauder]

Shouldn't we just convert an Iterable to a Collection if it isn't one already?