Skip to content

Commit fafcb85

Browse files
philippe-granetphilippe-granet
authored
Fix performance regression in AssertJ recipes (#505)
* Fix performance regression in AssertJ recipes Since commit 368384a , AssertJ recipes are really slower since calling contextSensitive() method disable a cache on JavaTemplate. Fixes: - Make sure there is a static import org.assertj.core.api.Assertions.* , even if not referenced (see #491 and #479) - reintroduced assertionsParser cache in JUnitAssertEqualsToAssertThat * Fix performance regression in AssertJ recipes Since commit 368384a , AssertJ recipes are really slower since calling contextSensitive() method disable a cache on JavaTemplate. Fixes: - Make sure there is a static import org.assertj.core.api.Assertions.* , even if not referenced (see #491 and #479) - reintroduced assertionsParser cache in JUnitAssertEqualsToAssertThat * Fix performance regression in AssertJ recipes Since commit 368384a , AssertJ recipes are really slower since calling contextSensitive() method disable a cache on JavaTemplate. Fixes: - Make sure there is a static import org.assertj.core.api.Assertions.* , even if not referenced (see #491 and #479) - reintroduced assertionsParser cache in JUnitAssertEqualsToAssertThat
1 parent d73c390 commit fafcb85

11 files changed

+54
-38
lines changed

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertArrayEqualsToAssertThat.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
7373
Expression expected = args.get(0);
7474
Expression actual = args.get(1);
7575

76-
maybeAddImport("org.assertj.core.api.Assertions", "assertThat");
76+
// Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat" (even if not referenced)
77+
maybeAddImport("org.assertj.core.api.Assertions", "assertThat", false);
7778
maybeRemoveImport(JUNIT_QUALIFIED_ASSERTIONS_CLASS_NAME);
7879

7980
if (args.size() == 2) {
@@ -93,7 +94,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
9394
.build()
9495
.apply(getCursor(), method.getCoordinates().replace(), actual, message, expected);
9596
} else if (args.size() == 3) {
96-
maybeAddImport("org.assertj.core.api.Assertions", "within");
97+
maybeAddImport("org.assertj.core.api.Assertions", "within", false);
9798
// assert is using floating points with a delta and no message.
9899
return JavaTemplate.builder("assertThat(#{anyArray()}).containsExactly(#{anyArray()}, within(#{any()}));")
99100
.staticImports("org.assertj.core.api.Assertions.assertThat", "org.assertj.core.api.Assertions.within")
@@ -104,7 +105,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
104105

105106
// The assertEquals is using a floating point with a delta argument and a message.
106107
Expression message = args.get(3);
107-
maybeAddImport("org.assertj.core.api.Assertions", "within");
108+
maybeAddImport("org.assertj.core.api.Assertions", "within", false);
108109

109110
JavaTemplate.Builder template = TypeUtils.isString(message.getType()) ?
110111
JavaTemplate.builder("assertThat(#{anyArray()}).as(#{any(String)}).containsExactly(#{anyArray()}, within(#{any()}));") :

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertEqualsToAssertThat.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,16 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
4949
}
5050

5151
public static class AssertEqualsToAssertThatVisitor extends JavaIsoVisitor<ExecutionContext> {
52+
private JavaParser.Builder<?, ?> assertionsParser;
53+
54+
private JavaParser.Builder<?, ?> assertionsParser(ExecutionContext ctx) {
55+
if (assertionsParser == null) {
56+
assertionsParser = JavaParser.fromJavaVersion()
57+
.classpathFromResources(ctx, "assertj-core-3.24");
58+
}
59+
return assertionsParser;
60+
}
61+
5262
private static final MethodMatcher JUNIT_ASSERT_EQUALS = new MethodMatcher("org.junit.jupiter.api.Assertions" + " assertEquals(..)");
5363

5464
@Override
@@ -63,13 +73,14 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
6373

6474
//always add the import (even if not referenced)
6575
maybeAddImport("org.assertj.core.api.Assertions", "assertThat", false);
76+
77+
// Remove import for "org.junit.jupiter.api.Assertions" if no longer used.
6678
maybeRemoveImport("org.junit.jupiter.api.Assertions");
6779

6880
if (args.size() == 2) {
6981
return JavaTemplate.builder("assertThat(#{any()}).isEqualTo(#{any()});")
70-
.contextSensitive()
7182
.staticImports("org.assertj.core.api.Assertions.assertThat")
72-
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "assertj-core-3.24"))
83+
.javaParser(assertionsParser(ctx))
7384
.build()
7485
.apply(getCursor(), method.getCoordinates().replace(), actual, expected);
7586
} else if (args.size() == 3 && !isFloatingPointType(args.get(2))) {
@@ -78,10 +89,9 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
7889
JavaTemplate.builder("assertThat(#{any()}).as(#{any(String)}).isEqualTo(#{any()});") :
7990
JavaTemplate.builder("assertThat(#{any()}).as(#{any(java.util.function.Supplier)}).isEqualTo(#{any()});");
8091
return template
81-
.contextSensitive()
8292
.staticImports("org.assertj.core.api.Assertions.assertThat")
8393
.imports("java.util.function.Supplier")
84-
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "assertj-core-3.24"))
94+
.javaParser(assertionsParser(ctx))
8595
.build()
8696
.apply(
8797
getCursor(),
@@ -91,11 +101,11 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
91101
expected
92102
);
93103
} else if (args.size() == 3) {
94-
maybeAddImport("org.assertj.core.api.Assertions", "within");
104+
//always add the import (even if not referenced)
105+
maybeAddImport("org.assertj.core.api.Assertions", "within", false);
95106
return JavaTemplate.builder("assertThat(#{any()}).isCloseTo(#{any()}, within(#{any()}));")
96-
.contextSensitive()
97107
.staticImports("org.assertj.core.api.Assertions.assertThat", "org.assertj.core.api.Assertions.within")
98-
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "assertj-core-3.24"))
108+
.javaParser(assertionsParser(ctx))
99109
.build()
100110
.apply(getCursor(), method.getCoordinates().replace(), actual, expected, args.get(2));
101111

@@ -104,15 +114,15 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
104114
// The assertEquals is using a floating point with a delta argument and a message.
105115
Expression message = args.get(3);
106116

107-
maybeAddImport("org.assertj.core.api.Assertions", "within");
117+
//always add the import (even if not referenced)
118+
maybeAddImport("org.assertj.core.api.Assertions", "within", false);
108119
JavaTemplate.Builder template = TypeUtils.isString(message.getType()) ?
109120
JavaTemplate.builder("assertThat(#{any()}).as(#{any(String)}).isCloseTo(#{any()}, within(#{any()}));") :
110121
JavaTemplate.builder("assertThat(#{any()}).as(#{any(java.util.function.Supplier)}).isCloseTo(#{any()}, within(#{any()}));");
111122
return template
112-
.contextSensitive()
113123
.staticImports("org.assertj.core.api.Assertions.assertThat", "org.assertj.core.api.Assertions.within")
114124
.imports("java.util.function.Supplier")
115-
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "assertj-core-3.24"))
125+
.javaParser(assertionsParser(ctx))
116126
.build()
117127
.apply(
118128
getCursor(),

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertFalseToAssertThat.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,10 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
9797
);
9898
}
9999

100-
maybeAddImport("org.assertj.core.api.Assertions", "assertThat");
100+
//Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat" (even if not referenced)
101+
maybeAddImport("org.assertj.core.api.Assertions", "assertThat", false);
102+
103+
// Remove import for "org.junit.jupiter.api.Assertions" if no longer used.
101104
maybeRemoveImport("org.junit.jupiter.api.Assertions");
102105

103106
return method;

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertNotEqualsToAssertThat.java‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
7474

7575
if (args.size() == 2) {
7676
method = JavaTemplate.builder("assertThat(#{any()}).isNotEqualTo(#{any()});")
77-
.contextSensitive()
7877
.staticImports("org.assertj.core.api.Assertions.assertThat")
7978
.javaParser(assertionsParser(ctx))
8079
.build()
@@ -93,7 +92,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
9392

9493

9594
method = template
96-
.contextSensitive()
9795
.staticImports("org.assertj.core.api.Assertions.assertThat")
9896
.javaParser(assertionsParser(ctx))
9997
.build()
@@ -106,7 +104,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
106104
);
107105
} else if (args.size() == 3) {
108106
method = JavaTemplate.builder("assertThat(#{any()}).isNotCloseTo(#{any()}, within(#{any()}));")
109-
.contextSensitive()
110107
.staticImports("org.assertj.core.api.Assertions.assertThat", "org.assertj.core.api.Assertions.within")
111108
.javaParser(assertionsParser(ctx))
112109
.build()
@@ -117,7 +114,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
117114
expected,
118115
args.get(2)
119116
);
120-
maybeAddImport("org.assertj.core.api.Assertions", "within");
117+
maybeAddImport("org.assertj.core.api.Assertions", "within", false);
121118
} else {
122119
Expression message = args.get(3);
123120

@@ -126,7 +123,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
126123
JavaTemplate.builder("assertThat(#{any()}).as(#{any(java.util.function.Supplier)}).isNotCloseTo(#{any()}, within(#{any()}));");
127124

128125
method = template
129-
.contextSensitive()
130126
.staticImports("org.assertj.core.api.Assertions.assertThat", "org.assertj.core.api.Assertions.within")
131127
.javaParser(assertionsParser(ctx))
132128
.build()
@@ -139,12 +135,13 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
139135
args.get(2)
140136
);
141137

142-
maybeAddImport("org.assertj.core.api.Assertions", "within");
138+
maybeAddImport("org.assertj.core.api.Assertions", "within", false);
143139
}
144140

145-
//Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat"
146-
maybeAddImport("org.assertj.core.api.Assertions", "assertThat");
147-
//And if there are no longer references to the JUnit assertions class, we can remove the import.
141+
//Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat" (even if not referenced)
142+
maybeAddImport("org.assertj.core.api.Assertions", "assertThat", false);
143+
144+
// Remove import for "org.junit.jupiter.api.Assertions" if no longer used.
148145
maybeRemoveImport("org.junit.jupiter.api.Assertions");
149146

150147
return method;

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertNotNullToAssertThat.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
7171

7272
if (args.size() == 1) {
7373
method = JavaTemplate.builder("assertThat(#{any()}).isNotNull();")
74-
.contextSensitive()
7574
.staticImports("org.assertj.core.api.Assertions.assertThat")
7675
.javaParser(assertionsParser(ctx))
7776
.build()
@@ -89,7 +88,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
8988
JavaTemplate.builder("assertThat(#{any()}).as(#{any(java.util.function.Supplier)}).isNotNull();");
9089

9190
method = template
92-
.contextSensitive()
9391
.staticImports("org.assertj.core.api.Assertions.assertThat")
9492
.javaParser(assertionsParser(ctx))
9593
.build()
@@ -101,8 +99,11 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
10199
);
102100
}
103101

102+
//Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat" (even if not referenced)
103+
maybeAddImport("org.assertj.core.api.Assertions", "assertThat", false);
104+
105+
//And if there are no longer references to the JUnit assertions class, we can remove the import.
104106
maybeRemoveImport("org.junit.jupiter.api.Assertions");
105-
maybeAddImport("org.assertj.core.api.Assertions", "assertThat");
106107

107108
return method;
108109
}

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertNullToAssertThat.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
7171

7272
if (args.size() == 1) {
7373
method = JavaTemplate.builder("assertThat(#{any()}).isNull();")
74-
.contextSensitive()
7574
.staticImports("org.assertj.core.api.Assertions.assertThat")
7675
.javaParser(assertionsParser(ctx))
7776
.build()
@@ -88,7 +87,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
8887
JavaTemplate.builder("assertThat(#{any()}).as(#{any(java.util.function.Supplier)}).isNull();");
8988

9089
method = template
91-
.contextSensitive()
9290
.staticImports("org.assertj.core.api.Assertions.assertThat")
9391
.javaParser(assertionsParser(ctx))
9492
.build()
@@ -100,12 +98,12 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
10098
);
10199
}
102100

101+
// Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat" (even if not referenced)
102+
maybeAddImport("org.assertj.core.api.Assertions", "assertThat", false);
103+
103104
// Remove import for "org.junit.jupiter.api.Assertions" if no longer used.
104105
maybeRemoveImport("org.junit.jupiter.api.Assertions");
105106

106-
// Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat".
107-
maybeAddImport("org.assertj.core.api.Assertions", "assertThat");
108-
109107
return method;
110108
}
111109
}

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertSameToAssertThat.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
7272

7373
if (args.size() == 2) {
7474
method = JavaTemplate.builder("assertThat(#{any()}).isSameAs(#{any()});")
75-
.contextSensitive()
7675
.staticImports("org.assertj.core.api.Assertions.assertThat")
7776
.javaParser(assertionsParser(ctx))
7877
.build()
@@ -90,7 +89,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
9089
JavaTemplate.builder("assertThat(#{any()}).as(#{any(java.util.function.Supplier)}).isSameAs(#{any()});");
9190

9291
method = template
93-
.contextSensitive()
9492
.staticImports("org.assertj.core.api.Assertions.assertThat")
9593
.javaParser(assertionsParser(ctx))
9694
.build()
@@ -103,8 +101,11 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
103101
);
104102
}
105103

104+
// Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat" (even if not referenced)
105+
maybeAddImport("org.assertj.core.api.Assertions", "assertThat", false);
106+
107+
// Remove import for "org.junit.jupiter.api.Assertions" if no longer used.
106108
maybeRemoveImport("org.junit.jupiter.api.Assertions");
107-
maybeAddImport("org.assertj.core.api.Assertions", "assertThat");
108109

109110
return method;
110111
}

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionType.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ && getCursor().getParentTreeCursor().getValue() instanceof J.Block) {
8585
mi.getCoordinates().replace(),
8686
mi.getArguments().get(0), executable
8787
);
88-
maybeAddImport("org.assertj.core.api.AssertionsForClassTypes", "assertThatExceptionOfType");
88+
maybeAddImport("org.assertj.core.api.AssertionsForClassTypes", "assertThatExceptionOfType", false);
8989
maybeRemoveImport("org.junit.jupiter.api.Assertions.assertThrows");
9090
maybeRemoveImport("org.junit.jupiter.api.Assertions");
9191
}

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertTrueToAssertThat.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,10 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
9797
);
9898
}
9999

100-
maybeAddImport("org.assertj.core.api.Assertions", "assertThat");
100+
//Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat" (even if not referenced)
101+
maybeAddImport("org.assertj.core.api.Assertions", "assertThat", false);
102+
103+
// Remove import for "org.junit.jupiter.api.Assertions" if no longer used.
101104
maybeRemoveImport("org.junit.jupiter.api.Assertions");
102105

103106
return method;

‎src/main/java/org/openrewrite/java/testing/assertj/JUnitFailToAssertJFail.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
147147
method.getCoordinates().replace(),
148148
arguments.toArray()
149149
);
150-
maybeAddImport("org.assertj.core.api.Assertions", "fail");
150+
//Make sure there is a static import for "org.assertj.core.api.Assertions.assertThat" (even if not referenced)
151+
maybeAddImport("org.assertj.core.api.Assertions", "fail", false);
151152
return super.visitMethodInvocation(method, ctx);
152153
}
153154
}

0 commit comments

Comments
 (0)