Skip to content

Commit

Permalink
Fix quarkusio#5885: generics break panache repo enhancer
Browse files Browse the repository at this point in the history
Do not enhance generic/abstract repos
Use proper generics resolution when looking up the repository type
arguments
  • Loading branch information
FroMage committed Dec 16, 2019
1 parent 535e033 commit 53d3886
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.hibernate.orm.panache.deployment;

import java.lang.reflect.Modifier;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -82,9 +83,13 @@ void build(CombinedIndexBuildItem index,
// Skip PanacheRepository
if (classInfo.name().equals(DOTNAME_PANACHE_REPOSITORY))
continue;
if (skipRepository(classInfo))
continue;
daoClasses.add(classInfo.name().toString());
}
for (ClassInfo classInfo : index.getIndex().getAllKnownImplementors(DOTNAME_PANACHE_REPOSITORY)) {
if (skipRepository(classInfo))
continue;
daoClasses.add(classInfo.name().toString());
}
for (String daoClass : daoClasses) {
Expand All @@ -95,6 +100,7 @@ void build(CombinedIndexBuildItem index,
Set<String> modelClasses = new HashSet<>();
// Note that we do this in two passes because for some reason Jandex does not give us subtypes
// of PanacheEntity if we ask for subtypes of PanacheEntityBase
// NOTE: we don't skip abstract/generic entities because they still need accessors
for (ClassInfo classInfo : index.getIndex().getAllKnownSubclasses(DOTNAME_PANACHE_ENTITY_BASE)) {
// FIXME: should we really skip PanacheEntity or all MappedSuperClass?
if (classInfo.name().equals(DOTNAME_PANACHE_ENTITY))
Expand Down Expand Up @@ -122,4 +128,11 @@ void build(CombinedIndexBuildItem index,
}
}

private boolean skipRepository(ClassInfo classInfo) {
// we don't want to add methods to abstract/generic entities/repositories: they get added to bottom types
// which can't be either
return Modifier.isAbstract(classInfo.flags())
|| !classInfo.typeParameters().isEmpty();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,13 @@ private String recursivelyFindEntityTypeFromClass(DotName clazz, DotName reposit
return null;
}

final ClassInfo classByName = indexView.getClassByName(clazz);
for (org.jboss.jandex.Type type : classByName.interfaceTypes()) {
if (type.name().equals(repositoryDotName)) {
org.jboss.jandex.Type entityType = type.asParameterizedType().arguments().get(0);
return entityType.name().toString().replace('.', '/');
}
}

return recursivelyFindEntityTypeFromClass(classByName.superName(), repositoryDotName);
List<org.jboss.jandex.Type> typeParameters = io.quarkus.deployment.util.JandexUtil
.resolveTypeParameters(clazz, repositoryDotName, indexView);
if (typeParameters.isEmpty())
throw new IllegalStateException(
"Failed to find supertype " + repositoryDotName + " from entity class " + clazz);
org.jboss.jandex.Type entityType = typeParameters.get(0);
return entityType.name().toString().replace('.', '/');
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package io.quarkus.it.panache;

import io.quarkus.hibernate.orm.panache.PanacheRepository;

public abstract class Bug5885AbstractRepository<T> implements PanacheRepository<T> {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package io.quarkus.it.panache;

import javax.enterprise.context.ApplicationScoped;

@ApplicationScoped
public class Bug5885EntityRepository extends Bug5885AbstractRepository<Person> {
}
Original file line number Diff line number Diff line change
Expand Up @@ -987,4 +987,15 @@ public String testBug5274() {
bug5274EntityRepository.count();
return "OK";
}

@Inject
Bug5885EntityRepository bug5885EntityRepository;

@GET
@Path("5885")
@Transactional
public String testBug5885() {
bug5885EntityRepository.findById(1L);
return "OK";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ public void testBug5274() {
RestAssured.when().get("/test/5274").then().body(is("OK"));
}

@Test
public void testBug5885() {
RestAssured.when().get("/test/5885").then().body(is("OK"));
}

/**
* This test is disabled in native mode as there is no interaction with the quarkus integration test endpoint.
*/
Expand Down

0 comments on commit 53d3886

Please sign in to comment.