Do not assume org.openrewrite.dataTables messages preserve order in DataTableWatcher (#31)

* Show the issue with a unit test that randomly succeeds, but mostly fails.

* Use LinkedHashMap for org.openrewrite.dataTables message values for consistent ordering of entries

* Do not assume org.openrewrite.dataTables messages preserves order in DataTableWatcher

* Remove newline

* Switch to LinkedList for faster node removal

---------

Co-authored-by: Bryce Tompkins <[email protected]>

Author: Jenson3210

build.gradle.kts

@@ -27,6 +27,8 @@ dependencies {
     testImplementation("io.moderne:moderne-organizations-format:latest.release")
     testImplementation("org.openrewrite:rewrite-test")
     testImplementation("org.openrewrite:rewrite-java-21:${rewriteVersion}")
+    testImplementation(gradleApi())
+    testImplementation("org.openrewrite.gradle.tooling:model:${rewriteVersion}")
     testRuntimeOnly("junit:junit:4.+")
 }
 

src/main/java/io/moderne/devcenter/internal/DataTableRowWatcher.java

@@ -19,7 +19,7 @@
 import org.openrewrite.DataTable;
 import org.openrewrite.ExecutionContext;
 
-import java.util.ArrayList;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
@@ -29,22 +29,22 @@
 public class DataTableRowWatcher<Row> {
     private final DataTable<Row> dataTable;
     private final ExecutionContext ctx;
-
-    int startingRowCount;
+    private List<Row> startingRows;
 
     public void start() {
-        startingRowCount = getRows().size();
+        startingRows = getRows();
     }
 
     public List<Row> stop() {
         List<Row> rows = getRows();
-        return rows.subList(startingRowCount, rows.size());
+        rows.removeAll(startingRows);
+        return rows;
     }
 
     private List<Row> getRows() {
         Map<DataTable<?>, List<?>> dataTables = ctx.getMessage("org.openrewrite.dataTables", emptyMap());
         // TODO because two DataTable of the same type can be created
-        List<Row> rows = new ArrayList<>();
+        List<Row> rows = new LinkedList<>();
         for (Map.Entry<DataTable<?>, List<?>> dataTableEntry : dataTables.entrySet()) {
             if (dataTableEntry.getKey().getClass().equals(dataTable.getClass())) {
                 //noinspection unchecked

src/test/java/io/moderne/devcenter/DevCenterTest.java

@@ -16,6 +16,7 @@
 package io.moderne.devcenter;
 
 import io.moderne.devcenter.table.UpgradesAndMigrations;
+import org.intellij.lang.annotations.Language;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -37,6 +38,9 @@
 import static io.moderne.devcenter.JavaVersionUpgrade.Measure.Java8Plus;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.tuple;
+import static org.openrewrite.gradle.Assertions.buildGradle;
+import static org.openrewrite.gradle.toolingapi.Assertions.withToolingApi;
 import static org.openrewrite.java.Assertions.java;
 import static org.openrewrite.java.Assertions.version;
 
@@ -199,7 +203,7 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
                 // Load io.moderne.devcenter classes (including UpgradeMigrationCard) in this classloader
                 // to ensure they are different from the ones in the test classloader
                 if (name.startsWith("io.moderne.devcenter.") &&
-                    !name.startsWith("io.moderne.devcenter.DevCenter")) {
+                  !name.startsWith("io.moderne.devcenter.DevCenter")) {
                     // Get the resource path for the class
                     String resourcePath = name.replace('.', '/') + ".class";
                     try (var inputStream = getClass().getClassLoader().getResourceAsStream(resourcePath)) {
@@ -231,7 +235,7 @@ public String getDisplayName() {
               @Override
               public String getDescription() {
                   return "Simulates `DeclarativeRecipe`, which is parent loaded and contains " +
-                         "child loaded sub-recipes.";
+                    "child loaded sub-recipes.";
               }
 
               @Override
@@ -241,4 +245,68 @@ public List<Recipe> getRecipeList() {
           })
         );
     }
+
+    @Test
+    void devcenterWithMultipleLibraryUpgradeRecipesHasCorrectData() {
+        @Language("yaml") String recipe = """
+          type: specs.openrewrite.org/v1beta/recipe
+          name: io.moderne.devcenter.TwoLibraryUpgrades
+          displayName: Starter DevCenter library version upgrade card
+          description: Upgrade library versions.
+          recipeList:
+            - io.moderne.devcenter.LibraryUpgrade:
+                cardName: Move to Spring Boot 3.5.0
+                groupIdPattern: org.springframework.boot
+                artifactIdPattern: '*'
+                version: 3.5.0
+                upgradeRecipe: io.moderne.java.spring.boot3.UpgradeSpringBoot_3_5
+            - io.moderne.devcenter.LibraryUpgrade:
+                cardName: Move to commons collections 3.2.2
+                groupIdPattern: commons-collections
+                artifactIdPattern: commons-collections
+                version: 3.2.2
+          """;
+        rewriteRun(
+          spec ->
+            spec.recipeFromYaml(recipe, "io.moderne.devcenter.TwoLibraryUpgrades")
+              .beforeRecipe(withToolingApi())
+              .afterRecipe(after -> assertThat(after.getDataTableRows("io.moderne.devcenter.table.UpgradesAndMigrations"))
+                .extracting("card", "ordinal", "value", "currentMinimumVersion")
+                .containsExactly(
+                  tuple("Move to Spring Boot 3.5.0", 0, "Major", "2.7.18"),
+                  tuple("Move to commons collections 3.2.2", 0, "Major", "2.0")
+                )),
+          //language=Groovy
+          buildGradle(
+            """
+              plugins {
+                  id "java"
+                  id 'org.springframework.boot' version '2.7.18'
+                  id 'io.spring.dependency-management' version '1.1.7'
+              }
+              repositories {
+                  mavenCentral()
+              }
+              dependencies {
+                  implementation "org.springframework.boot:spring-boot-starter-web"
+                  implementation "commons-collections:commons-collections:2.0"
+              }
+              """,
+            """
+            plugins {
+                id "java"
+                id 'org.springframework.boot' version '2.7.18'
+                id 'io.spring.dependency-management' version '1.1.7'
+            }
+            repositories {
+                mavenCentral()
+            }
+            dependencies {
+                /*~~(org.springframework.boot:spring-boot:2.7.18,org.springframework.boot:spring-boot-autoconfigure:2.7.18,org.springframework.boot:spring-boot-starter-logging:2.7.18,org.springframework.boot:spring-boot-starter-web:2.7.18,org.springframework.boot:spring-boot-starter:2.7.18,org.springframework.boot:spring-boot-starter-tomcat:2.7.18,org.springframework.boot:spring-boot-starter-json:2.7.18)~~>*/implementation "org.springframework.boot:spring-boot-starter-web"
+                /*~~(commons-collections:commons-collections:2.0)~~>*/implementation "commons-collections:commons-collections:2.0"
+            }
+            """
+          )
+        );
+    }
 }