Skip to content

Commit 2ebe33b

Browse files
nbauernfeindnbauernfeind
authored
fix: DH-20139: Remove $CLASSNAME$ Substitution (deephaven#7132)
1 parent c626e04 commit 2ebe33b

14 files changed

Lines changed: 99 additions & 54 deletions

File tree

‎engine/context/src/test/java/io/deephaven/engine/context/TestQueryCompiler.java‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,12 @@ public class TestQueryCompiler {
3030
private final static long WAIT_BETWEEN_THREAD_START_MILLIS = 5;
3131
private final static long MINIMUM_DELAY_MILLIS = 100;
3232
private final static int NUM_COMPILE_TESTS = 10;
33-
private static final String CLASS_CODE;
3433

3534
private final static List<Throwable> raisedThrowables = new ArrayList<>();
3635

3736
// Two nearly-identical classes, so we can get an idea of how long it takes to compile one of them
38-
static {
39-
final StringBuilder testClassCode1 = new StringBuilder(" public class $CLASSNAME$ {");
37+
private static String generateClassBody(final String className) {
38+
final StringBuilder testClassCode1 = new StringBuilder(" public class " + className + " {");
4039
testClassCode1.append(" final static String testString = \"Hello World\\n\";");
4140

4241
// Simple static inner classes to generate two class files
@@ -55,7 +54,7 @@ public class TestQueryCompiler {
5554
}
5655

5756
testClassCode1.append(" }");
58-
CLASS_CODE = testClassCode1.toString();
57+
return testClassCode1.toString();
5958
}
6059

6160
@Rule
@@ -181,7 +180,7 @@ private void compile(boolean printDetails, final String className) {
181180
QueryCompilerRequest.builder()
182181
.description("Test Compile")
183182
.className(className)
184-
.classBody(CLASS_CODE)
183+
.classBody(generateClassBody(className))
185184
.packageNameRoot("io.deephaven.temp")
186185
.build());
187186
if (printDetails) {
@@ -201,7 +200,7 @@ private String printMillis(final long millis) {
201200
public void testSimpleCompile() throws Exception {
202201
final String program1Text = String.join(
203202
"\n",
204-
"public class $CLASSNAME$ {",
203+
"public class Test {",
205204
" public static void main (String [] args) {",
206205
" System.out.println (\"Hello, World?\");",
207206
" System.out.println (args.length);",
@@ -383,4 +382,34 @@ public void testBadCompile() {
383382
"Class names, 'InvalidClassArgument', are only accepted if annotation processing is explicitly requested",
384383
e.getMessage());
385384
}
385+
386+
@Test
387+
public void testVariableClassNameThrows() {
388+
final String goodProgram = String.join("\n",
389+
"public class $CLASSNAME$ {",
390+
" public static void main (String [] args) {",
391+
" }",
392+
"}");
393+
394+
QueryCompilerRequest[] requests = new QueryCompilerRequest[] {
395+
QueryCompilerRequest.builder()
396+
.description("Test Good Compile")
397+
.className("FineFormula")
398+
.classBody(goodProgram)
399+
.packageNameRoot("com.deephaven.test")
400+
.build(),
401+
};
402+
403+
// noinspection unchecked
404+
CompletionStageFuture.Resolver<Class<?>>[] resolvers =
405+
(CompletionStageFuture.Resolver<Class<?>>[]) new CompletionStageFuture.Resolver[] {
406+
CompletionStageFuture.make(),
407+
};
408+
409+
IllegalArgumentException e = org.junit.Assert.assertThrows(IllegalArgumentException.class,
410+
() -> ExecutionContext.getContext().getQueryCompiler().compile(requests, resolvers));
411+
org.junit.Assert.assertEquals("QueryCompiler's support of the $CLASSNAME$ variable has been removed " +
412+
"as the final class name affects the compiled byte code and therefore cannot be dynamically " +
413+
"replaced.", e.getMessage());
414+
}
386415
}

‎engine/table/build.gradle‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,7 @@ dependencies {
9494
spotless {
9595
java {
9696
// Exclude these codegen sample files from here so we don't even need a comment to disable formatting
97-
targetExclude 'src/test/java/io/deephaven/engine/table/impl/select/FilterKernel*Sample.java',
98-
'src/test/java/io/deephaven/engine/table/impl/select/Formula*Sample.java'
97+
targetExclude 'src/test/java/io/deephaven/engine/table/impl/select/sample*/**/*.java'
9998
}
10099
}
101100

‎engine/table/src/main/java/io/deephaven/engine/context/QueryCompilerImpl.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -666,8 +666,13 @@ private static String loadIdentifyingField(Class<?> c) {
666666
}
667667

668668
private static String makeFinalCode(String className, String classBody, String packageName) {
669+
if (classBody.contains("$CLASSNAME$")) {
670+
throw new IllegalArgumentException("QueryCompiler's support of the $CLASSNAME$ variable has been removed as"
671+
+ " the final class name affects the compiled byte code and therefore cannot be dynamically "
672+
+ "replaced.");
673+
}
674+
669675
final String joinedEscapedBody = createEscapedJoinedString(classBody);
670-
classBody = classBody.replaceAll("\\$CLASSNAME\\$", className);
671676
classBody = classBody.substring(0, classBody.lastIndexOf("}"));
672677
classBody += " public static String " + IDENTIFYING_FIELD_NAME + " = " + joinedEscapedBody + ";\n}";
673678
return "package " + packageName + ";\n" + classBody;

‎engine/table/src/main/java/io/deephaven/engine/table/impl/select/ConditionFilter.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import io.deephaven.engine.table.ColumnSource;
2626
import io.deephaven.chunk.*;
2727
import io.deephaven.engine.rowset.chunkattributes.OrderedRowKeys;
28-
import io.deephaven.util.SafeCloseable;
2928
import io.deephaven.util.SafeCloseableList;
3029
import io.deephaven.util.text.Indenter;
3130
import io.deephaven.util.type.TypeUtils;
@@ -50,6 +49,8 @@
5049
*/
5150
public class ConditionFilter extends AbstractConditionFilter {
5251
public static final int CHUNK_SIZE = 4096;
52+
protected static final String CLASS_NAME = "GeneratedFilterKernel";
53+
5354
private Future<Class<?>> filterKernelClassFuture = null;
5455
private List<Pair<String, Class<?>>> usedInputs; // that is columns and special variables
5556
private String classBody;
@@ -456,7 +457,7 @@ protected void generateFilterCode(
456457

457458
filterKernelClassFuture = compilationProcessor.submit(QueryCompilerRequest.builder()
458459
.description("Filter Expression: " + formula)
459-
.className("GeneratedFilterKernel")
460+
.className(CLASS_NAME)
460461
.classBody(this.classBody)
461462
.packageNameRoot(QueryCompilerImpl.FORMULA_CLASS_PREFIX)
462463
.putAllParameterClasses(QueryScopeParamTypeUtil.expandParameterClasses(paramClasses))
@@ -491,8 +492,7 @@ private StringBuilder getClassBody(
491492
classBody
492493
.append(CodeGenerator
493494
.create(ExecutionContext.getContext().getQueryLibrary().getImportStrings().toArray()).build())
494-
.append(
495-
"\n\npublic class $CLASSNAME$ implements ")
495+
.append("\n\npublic class ").append(CLASS_NAME).append(" implements ")
496496
.append(FilterKernel.class.getCanonicalName()).append("<FilterKernel.Context>{\n");
497497
classBody.append("\n").append(timeConversionResult.getInstanceVariablesString()).append("\n");
498498
final Indenter indenter = new Indenter();
@@ -532,8 +532,8 @@ private StringBuilder getClassBody(
532532
classBody.append("\n");
533533
}
534534

535-
classBody.append("\n").append(indenter)
536-
.append("public $CLASSNAME$(Table __table, RowSet __fullSet, QueryScopeParam... __params) {\n");
535+
classBody.append("\n").append(indenter).append("public ").append(CLASS_NAME)
536+
.append("(Table __table, RowSet __fullSet, QueryScopeParam... __params) {\n");
537537
indenter.increaseLevel();
538538
for (int i = 0; i < params.length; i++) {
539539
final QueryScopeParam<?> param = params[i];

‎engine/table/src/main/java/io/deephaven/engine/table/impl/select/DhFormulaColumn.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public class DhFormulaColumn extends AbstractFormulaColumn {
5656
private static final String FORMULA_FACTORY_NAME = "__FORMULA_FACTORY";
5757
private static final String PARAM_CLASSNAME = QueryScopeParam.class.getCanonicalName();
5858
private static final String EVALUATION_EXCEPTION_CLASSNAME = FormulaEvaluationException.class.getCanonicalName();
59+
private static final String FORMULA_CLASS_NAME = "Formula";
5960
public static boolean useKernelFormulasProperty =
6061
Configuration.getInstance().getBooleanWithDefault("FormulaColumn.useKernelFormulasProperty", false);
6162

@@ -242,7 +243,7 @@ String generateClassBody() {
242243

243244
final CodeGenerator g = CodeGenerator.create(
244245
CodeGenerator.create(ExecutionContext.getContext().getQueryLibrary().getImportStrings().toArray()), "",
245-
"public class $CLASSNAME$ extends [[FORMULA_CLASS_NAME]]", CodeGenerator.block(
246+
"public class " + FORMULA_CLASS_NAME + " extends [[FORMULA_CLASS_NAME]]", CodeGenerator.block(
246247
generateFormulaFactoryLambda(), "",
247248
"private final String __columnName;",
248249
CodeGenerator.repeated("instanceVar", "private final [[TYPE]] [[NAME]];"),
@@ -288,15 +289,15 @@ String generateClassBody() {
288289

289290
private CodeGenerator generateFormulaFactoryLambda() {
290291
final CodeGenerator g = CodeGenerator.create(
291-
"public static final [[FORMULA_FACTORY]] [[FORMULA_FACTORY_NAME]] = $CLASSNAME$::new;");
292+
"public static final [[FORMULA_FACTORY]] [[FORMULA_FACTORY_NAME]] = " + FORMULA_CLASS_NAME + "::new;");
292293
g.replace("FORMULA_FACTORY", FormulaFactory.class.getCanonicalName());
293294
g.replace("FORMULA_FACTORY_NAME", FORMULA_FACTORY_NAME);
294295
return g.freeze();
295296
}
296297

297298
private CodeGenerator generateConstructor() {
298299
final CodeGenerator g = CodeGenerator.create(
299-
"public $CLASSNAME$(final String __columnName,", CodeGenerator.indent(
300+
"public " + FORMULA_CLASS_NAME + "(final String __columnName,", CodeGenerator.indent(
300301
"final TrackingRowSet __rowSet,",
301302
"final boolean __lazy,",
302303
"final java.util.Map<String, ? extends [[COLUMN_SOURCE_CLASSNAME]]> __columnsToData,",
@@ -779,7 +780,6 @@ public String getShiftedFormulaString() {
779780

780781
private void compileFormula(@NotNull final QueryCompilerRequestProcessor compilationRequestProcessor) {
781782
final String what = "Compile regular formula: " + formulaString;
782-
final String className = "Formula";
783783
final String classBody = generateClassBody();
784784

785785
final List<Class<?>> paramClasses = new ArrayList<>();
@@ -806,7 +806,7 @@ private void compileFormula(@NotNull final QueryCompilerRequestProcessor compila
806806

807807
formulaFactoryFuture = compilationRequestProcessor.submit(QueryCompilerRequest.builder()
808808
.description("Formula Expression: " + formulaString)
809-
.className(className)
809+
.className(FORMULA_CLASS_NAME)
810810
.classBody(classBody)
811811
.packageNameRoot(QueryCompilerImpl.FORMULA_CLASS_PREFIX)
812812
.putAllParameterClasses(QueryScopeParamTypeUtil.expandParameterClasses(paramClasses))

‎engine/table/src/main/java/io/deephaven/engine/table/impl/select/codegen/JavaKernelBuilder.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
public class JavaKernelBuilder {
3232
private static final String FORMULA_KERNEL_FACTORY_NAME = "__FORMULA_KERNEL_FACTORY";
33+
private static final String CLASS_NAME = "FormulaKernel";
3334

3435
public static CompletionStageFuture<Result> create(
3536
@NotNull final String originalFormulaString,
@@ -47,7 +48,7 @@ public static CompletionStageFuture<Result> create(
4748

4849
return compilationRequestProcessor.submit(QueryCompilerRequest.builder()
4950
.description("FormulaKernel: " + originalFormulaString)
50-
.className("Formula")
51+
.className(CLASS_NAME)
5152
.classBody(classBody)
5253
.packageNameRoot(QueryCompilerImpl.FORMULA_CLASS_PREFIX)
5354
.build()).thenApply(clazz -> {
@@ -105,7 +106,8 @@ private String generateKernelClassBody() {
105106

106107
final CodeGenerator g = CodeGenerator.create(
107108
CodeGenerator.create(ExecutionContext.getContext().getQueryLibrary().getImportStrings().toArray()), "",
108-
"public class $CLASSNAME$ implements [[FORMULA_KERNEL_INTERFACE_CANONICAL]]", CodeGenerator.block(
109+
"public class " + CLASS_NAME + " implements [[FORMULA_KERNEL_INTERFACE_CANONICAL]]",
110+
CodeGenerator.block(
109111
generateFactoryLambda(), "",
110112
CodeGenerator.repeated("instanceVar", "private final [[TYPE]] [[NAME]];"),
111113
timeInstanceVariables,
@@ -133,15 +135,16 @@ private String generateKernelClassBody() {
133135

134136
private CodeGenerator generateFactoryLambda() {
135137
final CodeGenerator g = CodeGenerator.create(
136-
"public static final [[FORMULA_KERNEL_FACTORY_CANONICAL]] [[FORMULA_KERNEL_FACTORY_NAME]] = $CLASSNAME$::new;");
138+
"public static final [[FORMULA_KERNEL_FACTORY_CANONICAL]] [[FORMULA_KERNEL_FACTORY_NAME]] = "
139+
+ CLASS_NAME + "::new;");
137140
g.replace("FORMULA_KERNEL_FACTORY_CANONICAL", FormulaKernelFactory.class.getCanonicalName());
138141
g.replace("FORMULA_KERNEL_FACTORY_NAME", FORMULA_KERNEL_FACTORY_NAME);
139142
return g.freeze();
140143
}
141144

142145
private CodeGenerator generateKernelConstructor() {
143146
final CodeGenerator g = CodeGenerator.create(
144-
"public $CLASSNAME$([[VECTOR_CANONICAL]][] __vectors,", CodeGenerator.indent(
147+
"public " + CLASS_NAME + "([[VECTOR_CANONICAL]][] __vectors,", CodeGenerator.indent(
145148
"[[PARAM_CANONICAL]][] __params)"),
146149
CodeGenerator.block(
147150
CodeGenerator.repeated("getVector", "[[NAME]] = ([[TYPE]])__vectors[[[INDEX]]];"),

‎engine/table/src/main/java/io/deephaven/engine/util/DynamicCompileUtils.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public static <T> Supplier<T> compileSimpleStatement(final Class<? extends T> re
3636
public static <T> Supplier<T> compileSimpleFunction(final Class<? extends T> resultType, final String code,
3737
final Collection<Class<?>> imports, final Collection<Class<?>> staticImports) {
3838
final StringBuilder classBody = new StringBuilder();
39+
final String className = "Function";
3940

4041
classBody.append("import ").append(resultType.getName()).append(";\n");
4142
for (final Class<?> im : imports) {
@@ -45,7 +46,8 @@ public static <T> Supplier<T> compileSimpleFunction(final Class<? extends T> res
4546
classBody.append("import static ").append(sim.getName()).append(".*;\n");
4647
}
4748

48-
classBody.append("public class $CLASSNAME$ implements ").append(Supplier.class.getCanonicalName()).append("<")
49+
classBody.append("public class " + className + " implements ").append(Supplier.class.getCanonicalName())
50+
.append("<")
4951
.append(resultType.getCanonicalName()).append(">").append(" ").append("{\n");
5052
classBody.append(" @Override\n");
5153
classBody.append(" public ").append(resultType.getCanonicalName()).append(" get() {\n");
@@ -56,7 +58,7 @@ public static <T> Supplier<T> compileSimpleFunction(final Class<? extends T> res
5658
final Class<?> partitionClass = ExecutionContext.getContext().getQueryCompiler().compile(
5759
QueryCompilerRequest.builder()
5860
.description("Simple Function: " + code)
59-
.className("Function")
61+
.className(className)
6062
.classBody(classBody.toString())
6163
.packageNameRoot(QueryCompilerImpl.FORMULA_CLASS_PREFIX)
6264
.build());
@@ -71,7 +73,9 @@ public static <T> Supplier<T> compileSimpleFunction(final Class<? extends T> res
7173

7274
public static Class<?> getClassThroughCompilation(final String object) {
7375
final StringBuilder classBody = new StringBuilder();
74-
classBody.append("public class $CLASSNAME$ implements ").append(Supplier.class.getCanonicalName())
76+
final String className = "Function";
77+
78+
classBody.append("public class " + className + " implements ").append(Supplier.class.getCanonicalName())
7579
.append("<Class>{ \n");
7680
classBody.append(" @Override\n");
7781
classBody.append(" public Class get() { return ").append(object).append(".class; }\n");
@@ -80,7 +84,7 @@ public static Class<?> getClassThroughCompilation(final String object) {
8084
final Class<?> partitionClass = ExecutionContext.getContext().getQueryCompiler().compile(
8185
QueryCompilerRequest.builder()
8286
.description("Formula: return " + object + ".class")
83-
.className("Function")
87+
.className(className)
8488
.classBody(classBody.toString())
8589
.packageNameRoot(QueryCompilerImpl.FORMULA_CLASS_PREFIX)
8690
.build());

‎engine/table/src/test/java/io/deephaven/engine/table/impl/select/TestConditionFilterGeneration.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package io.deephaven.engine.table.impl.select;
55

66
import io.deephaven.engine.context.ExecutionContext;
7+
import io.deephaven.engine.context.QueryCompilerImpl;
78
import io.deephaven.engine.table.Table;
89
import io.deephaven.engine.util.TableTools;
910
import io.deephaven.engine.table.impl.util.ModelFileGenerator;
@@ -40,22 +41,26 @@ public void tearDown() {
4041

4142
// @Test
4243
public void generateFile() throws FileNotFoundException {
43-
new ModelFileGenerator(FilterKernelSample.class).generateFile(getClassDefString());
44+
new ModelFileGenerator(io.deephaven.engine.table.impl.select.sample.GeneratedFilterKernel.class)
45+
.generateFile(getClassDefString());
4446
}
4547

4648
// @Test
4749
public void generateArrayFile() throws FileNotFoundException {
48-
new ModelFileGenerator(FilterKernelArraySample.class).generateFile(getArrayClassDefString());
50+
new ModelFileGenerator(io.deephaven.engine.table.impl.select.sample2.GeneratedFilterKernel.class)
51+
.generateFile(getArrayClassDefString());
4952
}
5053

5154
@Test
5255
public void validateFile() throws IOException {
53-
new ModelFileGenerator(FilterKernelSample.class).validateFile(getClassDefString());
56+
new ModelFileGenerator(io.deephaven.engine.table.impl.select.sample.GeneratedFilterKernel.class)
57+
.validateFile(getClassDefString());
5458
}
5559

5660
@Test
5761
public void validateArrayFile() throws IOException {
58-
new ModelFileGenerator(FilterKernelArraySample.class).validateFile(getArrayClassDefString());
62+
new ModelFileGenerator(io.deephaven.engine.table.impl.select.sample2.GeneratedFilterKernel.class)
63+
.validateFile(getArrayClassDefString());
5964
}
6065

6166
@NotNull

‎engine/table/src/test/java/io/deephaven/engine/table/impl/select/TestFormulaColumnGeneration.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import io.deephaven.engine.context.ExecutionContext;
77
import io.deephaven.engine.table.Table;
88
import io.deephaven.engine.context.QueryScope;
9+
import io.deephaven.engine.table.impl.select.sample.Formula;
10+
import io.deephaven.engine.table.impl.select.sample.FormulaKernel;
911
import io.deephaven.engine.util.TableTools;
1012
import io.deephaven.engine.table.impl.util.ModelFileGenerator;
1113
import io.deephaven.engine.testutil.junit4.EngineCleanup;
@@ -34,8 +36,8 @@ public class TestFormulaColumnGeneration {
3436
// @Test
3537
public void generateFiles() throws FileNotFoundException {
3638
final DhFormulaColumn fc = (DhFormulaColumn) getFormulaColumn();
37-
new ModelFileGenerator(FormulaSample.class).generateFile(fc.generateClassBody());
38-
new ModelFileGenerator(FormulaKernelSample.class).generateFile(fc.generateKernelClassBody());
39+
new ModelFileGenerator(Formula.class).generateFile(fc.generateClassBody());
40+
new ModelFileGenerator(FormulaKernel.class).generateFile(fc.generateKernelClassBody());
3941
}
4042

4143
@Rule
@@ -60,8 +62,8 @@ public void tearDown() {
6062
@Test
6163
public void validateFiles() throws IOException {
6264
final DhFormulaColumn fc = (DhFormulaColumn) getFormulaColumn();
63-
new ModelFileGenerator(FormulaSample.class).validateFile(fc.generateClassBody());
64-
new ModelFileGenerator(FormulaKernelSample.class).validateFile(fc.generateKernelClassBody());
65+
new ModelFileGenerator(Formula.class).validateFile(fc.generateClassBody());
66+
new ModelFileGenerator(FormulaKernel.class).validateFile(fc.generateKernelClassBody());
6567
}
6668

6769
@NotNull

0 commit comments

Comments
 (0)