Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 16 additions & 2 deletions src/main/java/org/openrewrite/prethink/ExportContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -325,19 +325,33 @@ private List<ColumnInfo> getColumnInfo(DataTable<?> table, List<?> rows) {
* Aggregate rows from all DataTable instances of the same class into a single list per class.
* When multiple recipes produce the same DataTable type (e.g., FindNodeTestCoverage and
* FindTestCoverage both produce TestMapping), this ensures all rows are combined.
* <p>
* Results are ordered by the configured {@link #dataTables} list to ensure deterministic
* output regardless of {@link java.util.concurrent.ConcurrentHashMap} iteration order
* in {@link ExecutionContext#DATA_TABLES}.
*/
@SuppressWarnings("unchecked")
private void aggregateMatchingTables(Map<DataTable<?>, List<?>> allTables,
Map<String, DataTable<?>> tablesByFqn,
Map<String, List<Object>> rowsByFqn) {
// Collect into temporary maps first
Map<String, DataTable<?>> unordered = new HashMap<>();
Map<String, List<Object>> unorderedRows = new HashMap<>();
for (Map.Entry<DataTable<?>, List<?>> entry : allTables.entrySet()) {
String tableFqn = entry.getKey().getClass().getName();
if (dataTables.contains(tableFqn)) {
tablesByFqn.putIfAbsent(tableFqn, entry.getKey());
rowsByFqn.computeIfAbsent(tableFqn, k -> new ArrayList<>())
unordered.putIfAbsent(tableFqn, entry.getKey());
unorderedRows.computeIfAbsent(tableFqn, k -> new ArrayList<>())
.addAll(entry.getValue());
}
}
// Insert into output maps in dataTables config order for deterministic output
for (String fqn : dataTables) {
if (unordered.containsKey(fqn)) {
tablesByFqn.put(fqn, unordered.get(fqn));
rowsByFqn.put(fqn, unorderedRows.get(fqn));
}
}
}

private String tableToFilename(String tableFqn) {
Expand Down
78 changes: 78 additions & 0 deletions src/test/java/org/openrewrite/prethink/ExportContextTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@
import lombok.Getter;
import org.junit.jupiter.api.Test;
import org.openrewrite.*;
import org.openrewrite.prethink.table.CodingConventions;
import org.openrewrite.prethink.table.TestMapping;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
import static org.openrewrite.test.SourceSpecs.text;
Expand Down Expand Up @@ -259,4 +263,78 @@ void aggregatesRowsFromMultipleInstancesOfSameDataTable() {
)
);
}

/**
* A fake recipe that populates CodingConventions rows.
*/
@Getter
public static class PopulateCodingConventions extends Recipe {
transient CodingConventions codingConventions = new CodingConventions(this);

String displayName = "Populate coding conventions";
String description = "Populates CodingConventions data table.";

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new TreeVisitor<>() {
@Override
public Tree visit(Tree tree, ExecutionContext ctx) {
if (tree instanceof SourceFile sf &&
sf.getSourcePath().toString().endsWith("Foo.java")) {
codingConventions.insertRow(ctx, new CodingConventions.Row(
"naming",
"camelCase methods",
"getUserName()",
"high",
"project-wide"
));
}
return tree;
}
};
}
}

@Test
void aggregateMatchingTablesOrdersOutputByConfiguredDataTablesOrder() throws Exception {
ExportContext exportContext = new ExportContext(
"Test",
"desc",
"long desc",
// Config order: CodingConventions FIRST, TestMapping SECOND
List.of(
"org.openrewrite.prethink.table.CodingConventions",
"org.openrewrite.prethink.table.TestMapping"
)
);

// Simulate DATA_TABLES with TestMapping FIRST — reverse of config order.
// This is what ConcurrentHashMap may produce non-deterministically.
Map<DataTable<?>, List<?>> allTables = new LinkedHashMap<>();
TestMapping testMapping = new TestMapping(Recipe.noop());
CodingConventions codingConventions = new CodingConventions(Recipe.noop());
allTables.put(testMapping, List.of(new TestMapping.Row(
"src/test/java/FooTest.java", "com.example.FooTest", "testFoo()",
"src/main/java/Foo.java", "com.example.Foo", "foo()", null, null
)));
allTables.put(codingConventions, List.of(new CodingConventions.Row(
"naming", "camelCase", "getFoo()", "high", "project-wide"
)));

Map<String, DataTable<?>> tablesByFqn = new LinkedHashMap<>();
Map<String, List<Object>> rowsByFqn = new LinkedHashMap<>();

var method = ExportContext.class.getDeclaredMethod(
"aggregateMatchingTables", Map.class, Map.class, Map.class);
method.setAccessible(true);
method.invoke(exportContext, allTables, tablesByFqn, rowsByFqn);

// Without the fix: tablesByFqn follows allTables iteration order (TestMapping first)
// With the fix: tablesByFqn follows dataTables config order (CodingConventions first)
assertThat(new ArrayList<>(tablesByFqn.keySet())).containsExactly(
"org.openrewrite.prethink.table.CodingConventions",
"org.openrewrite.prethink.table.TestMapping"
);
}

}