From 656202316c74f6fdcfa6aeab09abc67eaf8026ff Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 20 Jul 2015 16:50:14 +0200 Subject: [PATCH] DATAJPA-763 - Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworked integration test to make it work on EclipseLink, too. Simply checking for new no joins doesn't work here as it even adds joins for plain attribute traversals using root.get(…). We're now using Mockito to verify the expected behavior on the interaction level instead of the Root's state. Removed superfluous finals. Added some Javadoc where necessary. Original pull request: #151. --- .../data/jpa/repository/query/QueryUtils.java | 17 ++++++++--- ...EclipseLinkQueryUtilsIntegrationTests.java | 29 ++----------------- .../query/QueryUtilsIntegrationTests.java | 25 +++++++++------- 3 files changed, 30 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index 2b9d643ba..a21d8fe80 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -546,15 +546,24 @@ public abstract class QueryUtils { return from.join(attribute, JoinType.LEFT); } + /** + * Return whether the given {@link From} contains a fetch declaration for the attribute with the given name. + * + * @param from the {@link From} to check for fetches. + * @param attribute the attribute name to check. + * @return + */ + private static boolean isAlreadyFetched(From from, String attribute) { - private static boolean isAlreadyFetched(final From from, final String attribute) { - for(final Fetch f : from.getFetches()) { - final boolean sameName = f.getAttribute().getName().equals(attribute); + for (Fetch f : from.getFetches()) { - if(sameName && f.getJoinType().equals(JoinType.LEFT)) { + boolean sameName = f.getAttribute().getName().equals(attribute); + + if (sameName && f.getJoinType().equals(JoinType.LEFT)) { return true; } } + return false; } } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java index 5c3127805..4b84acd30 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,35 +15,10 @@ */ package org.springframework.data.jpa.repository.query; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; - -import java.util.Set; - -import javax.persistence.criteria.Join; -import javax.persistence.criteria.JoinType; -import javax.persistence.criteria.Root; - -import org.springframework.data.jpa.domain.sample.Order; import org.springframework.test.context.ContextConfiguration; /** * @author Oliver Gierke */ @ContextConfiguration("classpath:eclipselink.xml") -public class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests { - - /** - * Required as EclipseLink generates an inner join for plain association traversal. - */ - @Override - protected void assertNoJoinRequestedForOptionalAssociation(Root root) { - - Set> joins = root.getJoins(); - assertThat(joins, hasSize(1)); - - Join join = joins.iterator().next(); - assertThat(join.getAttribute().getName(), is("manager")); - assertThat(join.getJoinType(), is(JoinType.INNER)); - } -} +public class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests {} diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java index 7e6e60c27..8a87e6274 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java @@ -17,8 +17,10 @@ package org.springframework.data.jpa.repository.query; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import static org.mockito.Mockito.*; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Set; @@ -40,9 +42,9 @@ import javax.persistence.spi.PersistenceProviderResolverHolder; import org.hibernate.ejb.HibernatePersistence; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; import org.springframework.data.jpa.domain.sample.Category; import org.springframework.data.jpa.domain.sample.Order; -import org.springframework.data.jpa.domain.sample.Product; import org.springframework.data.jpa.domain.sample.User; import org.springframework.data.mapping.PropertyPath; import org.springframework.test.context.ContextConfiguration; @@ -149,19 +151,22 @@ public class QueryUtilsIntegrationTests { * @see DATAJPA-763 */ @Test + @SuppressWarnings("unchecked") public void doesNotCreateAJoinForAlreadyFetchedAssociation() { - final CriteriaBuilder builder = em.getCriteriaBuilder(); - final CriteriaQuery query = builder.createQuery(Category.class); - final Root root = query.from(Category.class); - root.fetch("product", JoinType.LEFT); + CriteriaBuilder builder = em.getCriteriaBuilder(); + CriteriaQuery query = builder.createQuery(Category.class); - QueryUtils.toExpressionRecursively(root, PropertyPath.from("product", Category.class)); - assertThat(root.getJoins(), is(empty())); - } + Root root = query.from(Category.class); - protected void assertNoJoinRequestedForOptionalAssociation(Root root) { - assertThat(root.getJoins(), is(empty())); + Root mock = Mockito.mock(Root.class); + doReturn(root.getModel()).when(mock).getModel(); + doReturn(Collections.singleton(root.fetch("product", JoinType.LEFT))).when(mock).getFetches(); + + QueryUtils.toExpressionRecursively(mock, PropertyPath.from("product", Category.class)); + + verify(mock, times(1)).get("product"); + verify(mock, times(0)).join(Mockito.eq("product"), Mockito.any(JoinType.class)); } @Entity