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.

Sign up for GitHub

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

Resteasy: do not use Jandex's ClassInfo in a Set #8687

Merged
merged 1 commit into from
Apr 22, 2020
Merged

Resteasy: do not use Jandex's ClassInfo in a Set #8687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 21 additions & 16 deletions ...main/java/io/quarkus/resteasy/server/common/deployment/ResteasyServerCommonProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,19 +229,21 @@ public void build(

Map<DotName, ClassInfo> scannedResources = new HashMap<>();
Set<DotName> pathInterfaces = new HashSet<>();
Set<ClassInfo> withoutDefaultCtor = new HashSet<>();
Map<DotName, ClassInfo> withoutDefaultCtor = new HashMap<>();
for (AnnotationInstance annotation : allPaths) {
if (annotation.target().kind() == AnnotationTarget.Kind.CLASS) {
ClassInfo clazz = annotation.target().asClass();
if (!Modifier.isInterface(clazz.flags())) {
String className = clazz.name().toString();
if (!additionalPaths.contains(annotation)) { // scanned resources only contains real JAX-RS resources
scannedResources.putIfAbsent(clazz.name(), clazz);
}
reflectiveClass.produce(new ReflectiveClassBuildItem(true, true, className));
if (!withoutDefaultCtor.containsKey(clazz.name())) {
String className = clazz.name().toString();
if (!additionalPaths.contains(annotation)) { // scanned resources only contains real JAX-RS resources
scannedResources.putIfAbsent(clazz.name(), clazz);
}
reflectiveClass.produce(new ReflectiveClassBuildItem(true, true, className));
Comment on lines +238 to +242
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these be outside of the if (!withoutDefaultCtor.containsKey(clazz.name())) check?

Copy link
Member Author

@aloubyansky aloubyansky Apr 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's in withoutDefaultCtor it's already been processed, hasn't it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is certainly true, but the point of withoutDefaultCtor wasn't really to guard against that case.
I do understand what your checking here, it just seems odd to reuse withoutDefaultCtor to perform this check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could introduce another Set for that purpose.


if (!clazz.hasNoArgsConstructor()) {
withoutDefaultCtor.add(clazz);
if (!clazz.hasNoArgsConstructor()) {
withoutDefaultCtor.put(clazz.name(), clazz);
}
}
} else {
pathInterfaces.add(clazz.name());
Expand Down Expand Up @@ -393,24 +395,26 @@ void processPathInterfaceImplementors(CombinedIndexBuildItem combinedIndexBuildI
if (pathInterfaces.isEmpty()) {
return;
}
Set<ClassInfo> pathInterfaceImplementors = new HashSet<>();
Map<DotName, ClassInfo> pathInterfaceImplementors = new HashMap<>();
for (DotName iface : pathInterfaces) {
for (ClassInfo implementor : index.getAllKnownImplementors(iface)) {
pathInterfaceImplementors.add(implementor);
if (!pathInterfaceImplementors.containsKey(implementor.name())) {
pathInterfaceImplementors.put(implementor.name(), implementor);
}
}
}
if (!pathInterfaceImplementors.isEmpty()) {
AdditionalBeanBuildItem.Builder builder = AdditionalBeanBuildItem.builder()
.setDefaultScope(resteasyConfig.singletonResources ? BuiltinScope.SINGLETON.getName() : null)
.setUnremovable();
for (ClassInfo implementor : pathInterfaceImplementors) {
if (scopes.isScopeDeclaredOn(implementor)) {
for (Map.Entry<DotName, ClassInfo> implementor : pathInterfaceImplementors.entrySet()) {
if (scopes.isScopeDeclaredOn(implementor.getValue())) {
// It has a scope defined - just mark it as unremovable
unremovableBeans
.produce(new UnremovableBeanBuildItem(new BeanClassNameExclusion(implementor.name().toString())));
.produce(new UnremovableBeanBuildItem(new BeanClassNameExclusion(implementor.getKey().toString())));
} else {
// No built-in scope found - add as additional bean
builder.addBeanClass(implementor.name().toString());
builder.addBeanClass(implementor.getKey().toString());
}
}
additionalBeans.produce(builder.build());
Expand Down Expand Up @@ -565,7 +569,7 @@ private static void registerProviders(ResteasyDeployment deployment,
}

private static void generateDefaultConstructors(BuildProducer<BytecodeTransformerBuildItem> transformers,
Set<ClassInfo> withoutDefaultCtor,
Map<DotName, ClassInfo> withoutDefaultCtor,
List<AdditionalJaxRsResourceDefiningAnnotationBuildItem> additionalJaxRsResourceDefiningAnnotations) {

final Set<String> allowedAnnotationPrefixes = new HashSet<>(1 + additionalJaxRsResourceDefiningAnnotations.size());
Expand All @@ -581,7 +585,8 @@ private static void generateDefaultConstructors(BuildProducer<BytecodeTransforme
}
}

for (ClassInfo classInfo : withoutDefaultCtor) {
for (Map.Entry<DotName, ClassInfo> entry : withoutDefaultCtor.entrySet()) {
final ClassInfo classInfo = entry.getValue();
// keep it super simple - only generate default constructor is the object is a direct descendant of Object
if (!(classInfo.superClassType() != null && classInfo.superClassType().name().equals(DotNames.OBJECT))) {
return;
Expand Down
5 changes: 2 additions & 3 deletions ...sy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT;
import static io.quarkus.resteasy.deployment.SecurityTransformerUtils.hasSecurityAnnotation;

import java.util.HashSet;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import org.jboss.jandex.ClassInfo;
Expand Down Expand Up @@ -48,7 +47,7 @@ void setUpDenyAllJaxRs(CombinedIndexBuildItem index,
ResteasyDeploymentBuildItem resteasyDeployment,
BuildProducer<AdditionalSecuredClassesBuildIem> additionalSecuredClasses) {
if (config.denyJaxRs) {
Set<ClassInfo> classes = new HashSet<>();
final List<ClassInfo> classes = new ArrayList<>();

List<String> resourceClasses = resteasyDeployment.getDeployment().getScannedResourceClasses();
for (String className : resourceClasses) {
Expand Down