Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specification toPredicate usage is violated in SimpleJpaRepository::delete #3036

Closed
aryu-ki opened this issue Jun 22, 2023 · 3 comments · May be fixed by #3468
Closed

Specification toPredicate usage is violated in SimpleJpaRepository::delete #3036

aryu-ki opened this issue Jun 22, 2023 · 3 comments · May be fixed by #3468
Labels
status: duplicate A duplicate of another issue

Comments

@aryu-ki
Copy link

aryu-ki commented Jun 22, 2023

        @Override
	public long delete(Specification<T> spec) {

		CriteriaBuilder builder = this.em.getCriteriaBuilder();
		CriteriaDelete<T> delete = builder.createCriteriaDelete(getDomainClass());

		Predicate predicate = spec.toPredicate(delete.from(getDomainClass()), null, builder);

		if (predicate != null) {
			delete.where(predicate);
		}

		return this.em.createQuery(delete).executeUpdate();
	}

This is the definition of delete method in SimpleJpaRepository. Clearly, query param sent is null, but in Specification::toPredicate we have this doc:

         /**
	 * Creates a WHERE clause for a query of the referenced entity in form of a {@link Predicate} for the given
	 * {@link Root} and {@link CriteriaQuery}.
	 *
	 * @param root must not be {@literal null}.
	 * @param query must not be {@literal null}.
	 * @param criteriaBuilder must not be {@literal null}.
	 * @return a {@link Predicate}, may be {@literal null}.
	 */
	@Nullable
	Predicate toPredicate(Root<T> root, CriteriaQuery<?> query, CriteriaBuilder criteriaBuilder);
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 22, 2023
@mp911de
Copy link
Member

mp911de commented Jun 22, 2023

You're right. This is because CriteriaDelete is not a CriteriaQuery. See also #2936

@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Jun 22, 2023
@mp911de mp911de added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 22, 2023
@aryu-ki
Copy link
Author

aryu-ki commented Jun 22, 2023

@mp911de You should change documentation string of toPredicate if I'm right, because it's a confusing doc. I could find the issue fast, but in my code, there were no obvious signs of what was going on. Other developers might get confused as well. Who knows how much time is it gonna cost to someone. It's obvious that documentation is misleading! At least change it to 'should' instead of 'must' or provide some additional docs on exceptional cases

@ghostg00
Copy link

I also think that toPredicate's method documentation should be changed, It does cause confusion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants