Skip to content

Commit

Permalink
Avoid call ManagedType.getAttribute(segment) for Hibernate
Browse files Browse the repository at this point in the history
It doesn't return concrete type for generic with Hibernate:
```
java.lang.IllegalArgumentException: Unable to locate Attribute with the given name [name] on this ManagedType [java.lang.Object]
	at org.hibernate.metamodel.model.domain.AbstractManagedType.checkNotNull(AbstractManagedType.java:225) ~[hibernate-core-6.3.1.Final.jar:6.3.1.Final]
	at org.hibernate.metamodel.model.domain.AbstractManagedType.getAttribute(AbstractManagedType.java:148) ~[hibernate-core-6.3.1.Final.jar:6.3.1.Final]
	at org.hibernate.metamodel.model.domain.AbstractManagedType.getAttribute(AbstractManagedType.java:43) ~[hibernate-core-6.3.1.Final.jar:6.3.1.Final]
	at org.springframework.data.jpa.repository.query.QueryUtils.requiresOuterJoin(QueryUtils.java:836) ~[spring-data-jpa-3.2.0.jar:3.2.0]
```

Fix spring-projectsGH-3274
  • Loading branch information
quaff committed Dec 18, 2023
1 parent c9442a2 commit ba18571
Show file tree
Hide file tree
Showing 10 changed files with 275 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
* @author Vladislav Yukharin
* @author Chris Fraser
* @author Donghun Shin
* @author Yanming Zhou
*/
public abstract class QueryUtils {

Expand Down Expand Up @@ -821,20 +822,28 @@ private static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property,
Bindable<?> propertyPathModel;
Bindable<?> model = from.getModel();

// required for EclipseLink: we try to avoid using from.get as EclipseLink produces an inner join
// regardless of which join operation is specified next
// see: https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892
// still occurs as of 2.7
ManagedType<?> managedType = null;
if (model instanceof ManagedType) {
managedType = (ManagedType<?>) model;
} else if (model instanceof SingularAttribute
&& ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
managedType = (ManagedType<?>) ((SingularAttribute<?, ?>) model).getType();
}
if (managedType != null) {
propertyPathModel = (Bindable<?>) managedType.getAttribute(segment);
if (model.getClass().getName().startsWith("org.eclipse.persistence")) {
// required for EclipseLink: we try to avoid using from.get as EclipseLink produces an inner join
// regardless of which join operation is specified next
// see: https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892
// still occurs as of 2.7
ManagedType<?> managedType = null;
if (model instanceof ManagedType) {
managedType = (ManagedType<?>) model;
} else if (model instanceof SingularAttribute
&& ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
managedType = (ManagedType<?>) ((SingularAttribute<?, ?>) model).getType();
}
if (managedType != null) {
propertyPathModel = (Bindable<?>) managedType.getAttribute(segment);
} else {
propertyPathModel = from.get(segment).getModel();
}
} else {
// We shouldn't rely on managedType by default because it may be erased type if declared as generic
// see: https://hibernate.atlassian.net/browse/HHH-16144
// see: https://github.com/hibernate/hibernate-orm/pull/7630
// see: https://github.com/jakartaee/persistence/issues/562
propertyPathModel = from.get(segment).getModel();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright 2008-2023 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.jpa.domain.sample;

import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import org.springframework.data.jpa.domain.AbstractPersistable;

/**
* @author Yanming Zhou
*/
@Entity
public class Book extends OwnerContainer<Owner> {

@Id
@GeneratedValue
private Long id;

public Long getId() {
return id;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright 2008-2023 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.jpa.domain.sample;

import jakarta.persistence.Entity;
import org.springframework.data.jpa.domain.AbstractPersistable;

/**
* @author Yanming Zhou
*/
@Entity
public class Owner extends AbstractPersistable<Long> {

private String name;

public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2008-2023 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.jpa.domain.sample;

import jakarta.persistence.ManyToOne;
import jakarta.persistence.MappedSuperclass;

/**
* @author Yanming Zhou
*/
@MappedSuperclass
public class OwnerContainer<T> {

@ManyToOne
T owner;

public T getOwner() {
return owner;
}

public void setOwner(T owner) {
this.owner = owner;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright 2014-2023 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.jpa.repository.generics;

import jakarta.persistence.EntityManager;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.jpa.domain.sample.Book;
import org.springframework.data.jpa.domain.sample.Owner;
import org.springframework.data.jpa.repository.sample.BookRepository;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.transaction.annotation.Transactional;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Yanming Zhou
*/
@Transactional
@ExtendWith(SpringExtension.class)
@ContextConfiguration({ "classpath:eclipselink.xml", "classpath:config/namespace-application-context.xml" })
class EclipseLinkGenericsIntegrationTests extends GenericsIntegrationTests {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright 2014-2023 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.jpa.repository.generics;

import jakarta.persistence.EntityManager;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.jpa.domain.AbstractPersistable;
import org.springframework.data.jpa.domain.sample.Book;
import org.springframework.data.jpa.domain.sample.CustomAbstractPersistable;
import org.springframework.data.jpa.domain.sample.Owner;
import org.springframework.data.jpa.repository.sample.BookRepository;
import org.springframework.data.jpa.repository.sample.CustomAbstractPersistableRepository;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.transaction.annotation.Transactional;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Yanming Zhou
*/
@Transactional
@ExtendWith(SpringExtension.class)
@ContextConfiguration(locations = { "classpath:config/namespace-autoconfig-context.xml" })
class GenericsIntegrationTests {

@Autowired
BookRepository repository;

@Autowired
EntityManager entityManager;

@BeforeEach
void setUp() {
Owner owner = new Owner();
owner.setName("owner");
entityManager.persist(owner);
Book book = new Book();
book.setOwner(owner);
entityManager.persist(book);
}

@Test
void findAllByGenericAssociationProperty() {
assertThat(repository.findAllByOwnerName("owner")).hasSize(1);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static java.util.Collections.singletonList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand All @@ -34,6 +35,7 @@
import jakarta.persistence.criteria.From;
import jakarta.persistence.criteria.Join;
import jakarta.persistence.criteria.JoinType;
import jakarta.persistence.criteria.Path;
import jakarta.persistence.criteria.Root;
import jakarta.persistence.spi.PersistenceProvider;
import jakarta.persistence.spi.PersistenceProviderResolver;
Expand All @@ -54,6 +56,7 @@
import org.springframework.data.jpa.domain.sample.Invoice;
import org.springframework.data.jpa.domain.sample.InvoiceItem;
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.jpa.infrastructure.HibernateTestUtils;
import org.springframework.data.mapping.PropertyPath;
Expand All @@ -69,6 +72,7 @@
* @author Patrice Blanchardie
* @author Diego Krupitza
* @author Krzysztof Krason
* @author Yanming Zhou
*/
@ExtendWith(SpringExtension.class)
@ContextConfiguration("classpath:infrastructure.xml")
Expand Down Expand Up @@ -291,12 +295,14 @@ void doesNotCreateAJoinForAlreadyFetchedAssociation() {
Root<Category> root = query.from(Category.class);

Root<Category> mock = Mockito.mock(Root.class);
Path<Product> path = Mockito.mock(Path.class);
doReturn(root.getModel()).when(mock).getModel();
doReturn(path).when(mock).get("product");
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, atLeast(1)).get("product");
verify(mock, times(0)).join(Mockito.eq("product"), Mockito.any(JoinType.class));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright 2014-2023 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.jpa.repository.sample;

import org.springframework.data.jpa.domain.sample.Book;
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.List;

/**
* @author Yanming Zhou
*/
public interface BookRepository extends JpaRepository<Book, Long> {

List<Book> findAllByOwnerName(String ownerName);
}
2 changes: 2 additions & 0 deletions spring-data-jpa/src/test/resources/META-INF/persistence.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<class>org.springframework.data.jpa.domain.sample.AuditableUser</class>
<class>org.springframework.data.jpa.domain.sample.AuditableEntity</class>
<class>org.springframework.data.jpa.domain.sample.AuditableEmbeddable</class>
<class>org.springframework.data.jpa.domain.sample.Book</class>
<class>org.springframework.data.jpa.domain.sample.Category</class>
<class>org.springframework.data.jpa.domain.sample.Child</class>
<class>org.springframework.data.jpa.domain.sample.ConcreteType1</class>
Expand All @@ -33,6 +34,7 @@
<class>org.springframework.data.jpa.domain.sample.MailSender</class>
<class>org.springframework.data.jpa.domain.sample.MailUser</class>
<class>org.springframework.data.jpa.domain.sample.Order</class>
<class>org.springframework.data.jpa.domain.sample.Owner</class>
<class>org.springframework.data.jpa.domain.sample.Parent</class>
<class>org.springframework.data.jpa.domain.sample.PersistableWithIdClass</class>
<class>org.springframework.data.jpa.domain.sample.PersistableWithSingleIdClass
Expand Down
2 changes: 2 additions & 0 deletions spring-data-jpa/src/test/resources/META-INF/persistence2.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<class>org.springframework.data.jpa.domain.sample.AuditableUser</class>
<class>org.springframework.data.jpa.domain.sample.AuditableEntity</class>
<class>org.springframework.data.jpa.domain.sample.AuditableEmbeddable</class>
<class>org.springframework.data.jpa.domain.sample.Book</class>
<class>org.springframework.data.jpa.domain.sample.Category</class>
<class>org.springframework.data.jpa.domain.sample.CustomAbstractPersistable</class>
<class>org.springframework.data.jpa.domain.sample.EntityWithAssignedId</class>
Expand All @@ -20,6 +21,7 @@
<class>org.springframework.data.jpa.domain.sample.Role</class>
<class>org.springframework.data.jpa.domain.sample.Site</class>
<class>org.springframework.data.jpa.domain.sample.SpecialUser</class>
<class>org.springframework.data.jpa.domain.sample.Owner</class>
<class>org.springframework.data.jpa.domain.sample.User</class>
<class>org.springframework.data.jpa.domain.sample.Dummy</class>
<exclude-unlisted-classes>true</exclude-unlisted-classes>
Expand Down

0 comments on commit ba18571

Please sign in to comment.