diff --git a/querydsl-tooling/querydsl-apt/src/main/java/com/querydsl/apt/AbstractQuerydslProcessor.java b/querydsl-tooling/querydsl-apt/src/main/java/com/querydsl/apt/AbstractQuerydslProcessor.java index 03de30604c..3fef25816e 100644 --- a/querydsl-tooling/querydsl-apt/src/main/java/com/querydsl/apt/AbstractQuerydslProcessor.java +++ b/querydsl-tooling/querydsl-apt/src/main/java/com/querydsl/apt/AbstractQuerydslProcessor.java @@ -51,6 +51,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Deque; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -212,13 +213,7 @@ protected void processAnnotations() { // extend entity types typeFactory.extendTypes(); - if (!context.entityTypes.isEmpty()) { - var serializerConfig = - conf.getSerializerConfig(context.entityTypes.values().iterator().next()); - if (serializerConfig.createDefaultVariable()) { - detectCircularQClassReferences(); - } - } + detectCircularQClassReferences(); context.clean(); } @@ -691,70 +686,34 @@ private void serialize(Serializer serializer, Collection models) { } private void detectCircularQClassReferences() { - Map typeMap = context.entityTypes; - - List detectedCycles = new ArrayList<>(); - Set globalVisited = new HashSet<>(); - - for (EntityType start : typeMap.values()) { - if (globalVisited.contains(start.getFullName())) continue; - - Deque path = new ArrayDeque<>(); - Set inStack = new HashSet<>(); - dfs(start, typeMap, path, inStack, globalVisited, detectedCycles); - } - - if (!detectedCycles.isEmpty()) { - var message = new StringBuilder(); - message.append("[QueryDSL] Circular Q-class references detected.\n"); - message.append( - "This may cause class initialization deadlock in multi-threaded environments.\n\n"); - message.append("Detected cycles:\n"); - for (int i = 0; i < detectedCycles.size(); i++) { - message.append(" (").append(i + 1).append(") ").append(detectedCycles.get(i)).append("\n"); + Map entitiesWithDefaultVariable = new HashMap<>(); + for (var entry : context.entityTypes.entrySet()) { + if (conf.getSerializerConfig(entry.getValue()).createDefaultVariable()) { + entitiesWithDefaultVariable.put(entry.getKey(), entry.getValue()); } - message.append("\nTo avoid deadlock, consider:\n"); - message.append(" (1) Removing the bidirectional association on one side.\n"); - message.append( - " (2) Pre-initializing Q-classes in a single thread before handling requests (e.g. via @PostConstruct).\n"); - message.append( - " (3) Using 'new QClass(\"alias\")' instead of static field access in your repositories."); - - processingEnv.getMessager().printMessage(Kind.WARNING, message.toString()); } - } + List> detectedCycles = QClassCycleDetector.detect(entitiesWithDefaultVariable); + if (detectedCycles.isEmpty()) return; - private void dfs( - EntityType current, - Map typeMap, - Deque path, - Set inStack, - Set globalVisited, - List detectedCycles) { - - String currentName = current.getFullName(); - globalVisited.add(currentName); - inStack.add(currentName); - path.addLast(current.getSimpleName()); - - for (Property property : current.getProperties()) { - String neighborName = property.getType().getFullName(); - if (neighborName.equals(currentName)) continue; - - EntityType neighbor = typeMap.get(neighborName); - if (neighbor == null) continue; - - if (inStack.contains(neighborName)) { - List cycle = new ArrayList<>(path); - cycle.add(neighbor.getSimpleName()); - detectedCycles.add(String.join(" → ", cycle)); - } else if (!globalVisited.contains(neighborName)) { - dfs(neighbor, typeMap, path, inStack, globalVisited, detectedCycles); - } + var cyclesList = new StringBuilder(); + for (int i = 0; i < detectedCycles.size(); i++) { + cyclesList.append(" (%d) %s\n".formatted(i + 1, String.join(" → ", detectedCycles.get(i)))); } + var message = + """ + [QueryDSL] Circular Q-class references detected. + This may cause class initialization deadlock in multi-threaded environments. + + Detected cycles: + %s + To avoid deadlock, consider: + (1) Removing the bidirectional association on one side. + (2) Pre-initializing Q-classes in a single thread before handling requests (e.g. via @PostConstruct). + (3) Generating Q-classes without the static default variable (-Aquerydsl.createDefaultVariable=false) and using 'new QClass("alias")' instead.\ + """ + .formatted(cyclesList); - path.removeLast(); - inStack.remove(currentName); + processingEnv.getMessager().printMessage(Kind.WARNING, message); } protected String getClassName(EntityType model) { diff --git a/querydsl-tooling/querydsl-apt/src/main/java/com/querydsl/apt/QClassCycleDetector.java b/querydsl-tooling/querydsl-apt/src/main/java/com/querydsl/apt/QClassCycleDetector.java new file mode 100644 index 0000000000..93bdfa751b --- /dev/null +++ b/querydsl-tooling/querydsl-apt/src/main/java/com/querydsl/apt/QClassCycleDetector.java @@ -0,0 +1,83 @@ +/* + * Copyright 2015, The Querydsl Team (http://www.querydsl.com/team) + * + * 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 + * http://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 com.querydsl.apt; + +import com.querydsl.codegen.EntityType; +import com.querydsl.codegen.Property; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +final class QClassCycleDetector { + + private final Map typeMap; + private final List path = new ArrayList<>(); + private final Set inStack = new HashSet<>(); + private final Set globalVisited = new HashSet<>(); + private final List> cycles = new ArrayList<>(); + + private QClassCycleDetector(Map typeMap) { + this.typeMap = typeMap; + } + + static List> detect(Map typeMap) { + var detector = new QClassCycleDetector(typeMap); + var starts = new ArrayList<>(typeMap.values()); + starts.sort(Comparator.comparing(EntityType::getFullName)); + for (EntityType start : starts) { + if (!detector.globalVisited.contains(start.getFullName())) { + detector.visit(start); + } + } + return detector.cycles; + } + + private void visit(EntityType current) { + String currentName = current.getFullName(); + globalVisited.add(currentName); + inStack.add(currentName); + path.add(current); + + for (Property property : current.getProperties()) { + String neighborName = property.getType().getFullName(); + if (neighborName.equals(currentName)) continue; + + EntityType neighbor = typeMap.get(neighborName); + if (neighbor == null) continue; + + if (inStack.contains(neighborName)) { + cycles.add(sliceCycleFrom(neighbor)); + } else if (!globalVisited.contains(neighborName)) { + visit(neighbor); + } + } + + path.remove(path.size() - 1); + inStack.remove(currentName); + } + + private List sliceCycleFrom(EntityType entry) { + int start = path.indexOf(entry); + + List cycle = new ArrayList<>(path.size() - start + 1); + for (int i = start; i < path.size(); i++) { + cycle.add(path.get(i).getSimpleName()); + } + cycle.add(path.get(start).getSimpleName()); + return cycle; + } +} diff --git a/querydsl-tooling/querydsl-apt/src/test/java/com/querydsl/apt/QClassCycleDetectorTest.java b/querydsl-tooling/querydsl-apt/src/test/java/com/querydsl/apt/QClassCycleDetectorTest.java new file mode 100644 index 0000000000..2efebdc5c5 --- /dev/null +++ b/querydsl-tooling/querydsl-apt/src/test/java/com/querydsl/apt/QClassCycleDetectorTest.java @@ -0,0 +1,153 @@ +/* + * Copyright 2015, The Querydsl Team (http://www.querydsl.com/team) + * + * 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 + * http://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 com.querydsl.apt; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.querydsl.codegen.EntityType; +import com.querydsl.codegen.Property; +import com.querydsl.codegen.utils.model.SimpleType; +import com.querydsl.codegen.utils.model.TypeCategory; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class QClassCycleDetectorTest { + + @Test + void noReferences_returnsEmpty() { + var a = entity("A"); + var b = entity("B"); + + var cycles = QClassCycleDetector.detect(map(a, b)); + + assertThat(cycles).isEmpty(); + } + + @Test + void unidirectionalReference_returnsEmpty() { + var a = entity("A"); + var b = entity("B"); + reference(a, "b", b); + + var cycles = QClassCycleDetector.detect(map(a, b)); + + assertThat(cycles).isEmpty(); + } + + @Test + void selfReference_isIgnored() { + var a = entity("A"); + reference(a, "self", a); + + var cycles = QClassCycleDetector.detect(map(a)); + + assertThat(cycles).isEmpty(); + } + + @Test + void referenceToUnknownType_isIgnored() { + var a = entity("A"); + var external = simpleType("External"); + a.addProperty(new Property(a, "external", external)); + + var cycles = QClassCycleDetector.detect(map(a)); + + assertThat(cycles).isEmpty(); + } + + @Test + void twoNodeCycle_isDetected() { + var a = entity("A"); + var b = entity("B"); + reference(a, "b", b); + reference(b, "a", a); + + var cycles = QClassCycleDetector.detect(map(a, b)); + + assertThat(cycles).containsExactly(List.of("A", "B", "A")); + } + + @Test + void threeNodeCycle_isDetected() { + var a = entity("A"); + var b = entity("B"); + var c = entity("C"); + reference(a, "b", b); + reference(b, "c", c); + reference(c, "a", a); + + var cycles = QClassCycleDetector.detect(map(a, b, c)); + + assertThat(cycles).containsExactly(List.of("A", "B", "C", "A")); + } + + @Test + void multipleDisjointCycles_reportedInDeterministicOrder() { + var a = entity("A"); + var b = entity("B"); + var c = entity("C"); + var d = entity("D"); + reference(a, "b", b); + reference(b, "a", a); + reference(c, "d", d); + reference(d, "c", c); + + Map shuffled = new LinkedHashMap<>(); + shuffled.put(d.getFullName(), d); + shuffled.put(b.getFullName(), b); + shuffled.put(c.getFullName(), c); + shuffled.put(a.getFullName(), a); + + var cycles = QClassCycleDetector.detect(shuffled); + + assertThat(cycles).containsExactly(List.of("A", "B", "A"), List.of("C", "D", "C")); + } + + @Test + void cycleReachedFromOutside_reportsOnlyCycleNotEntryPath() { + var a = entity("A"); + var b = entity("B"); + var c = entity("C"); + reference(a, "b", b); + reference(b, "c", c); + reference(c, "b", b); + + var cycles = QClassCycleDetector.detect(map(a, b, c)); + + assertThat(cycles).containsExactly(List.of("B", "C", "B")); + } + + private static EntityType entity(String simpleName) { + return new EntityType(simpleType(simpleName)); + } + + private static SimpleType simpleType(String simpleName) { + return new SimpleType( + TypeCategory.ENTITY, "test." + simpleName, "test", simpleName, false, false); + } + + private static void reference(EntityType from, String propertyName, EntityType to) { + from.addProperty(new Property(from, propertyName, simpleType(to.getSimpleName()))); + } + + private static Map map(EntityType... entities) { + Map result = new LinkedHashMap<>(); + for (EntityType entity : entities) { + result.put(entity.getFullName(), entity); + } + return result; + } +}