fix(flyway): multiple datasource migration does not work on native

The QuarkusPathLocationScanner returned all migrations locations by default, this risked to cause collision when we have a same filename for a migration script which is handled by different datasources - a behaviour encountered on the issue quarkusio#7685. Let's avoid this case by making sure that the scanner only returns migration that it can recognize. Fixes quarkusio#7685
machi1990 · Mar 22, 2020 · ecddfb0 · ecddfb0
1 parent 3423ed6
commit ecddfb0
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 66 deletions.
diff --git a/extensions/flyway/deployment/src/main/java/io/quarkus/flyway/FlywayProcessor.java b/extensions/flyway/deployment/src/main/java/io/quarkus/flyway/FlywayProcessor.java
@@ -6,7 +6,6 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
@@ -39,15 +38,12 @@
 import io.quarkus.deployment.annotations.Record;
 import io.quarkus.deployment.builditem.CapabilityBuildItem;
 import io.quarkus.deployment.builditem.FeatureBuildItem;
-import io.quarkus.deployment.builditem.GeneratedResourceBuildItem;
 import io.quarkus.deployment.builditem.ServiceStartBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.NativeImageResourceBuildItem;
-import io.quarkus.deployment.recording.RecorderContext;
 import io.quarkus.flyway.runtime.FlywayBuildTimeConfig;
 import io.quarkus.flyway.runtime.FlywayProducer;
 import io.quarkus.flyway.runtime.FlywayRecorder;
 import io.quarkus.flyway.runtime.FlywayRuntimeConfig;
-import io.quarkus.flyway.runtime.graal.QuarkusPathLocationScanner;
 
 class FlywayProcessor {
 
@@ -72,11 +68,9 @@ void build(BuildProducer<AdditionalBeanBuildItem> additionalBeanProducer,
             BuildProducer<FeatureBuildItem> featureProducer,
             BuildProducer<NativeImageResourceBuildItem> resourceProducer,
             BuildProducer<BeanContainerListenerBuildItem> containerListenerProducer,
-            BuildProducer<GeneratedResourceBuildItem> generatedResourceProducer,
             FlywayRecorder recorder,
             List<JdbcDataSourceBuildItem> jdbcDataSourceBuildItems,
-            BuildProducer<GeneratedBeanBuildItem> generatedBeanBuildItem,
-            RecorderContext recorderContext) throws IOException, URISyntaxException {
+            BuildProducer<GeneratedBeanBuildItem> generatedBeanBuildItem) throws IOException, URISyntaxException {
 
         featureProducer.produce(new FeatureBuildItem(FeatureBuildItem.FLYWAY));
 
@@ -88,19 +82,20 @@ void build(BuildProducer<AdditionalBeanBuildItem> additionalBeanProducer,
                 .collect(Collectors.toSet());
         new FlywayDatasourceBeanGenerator(dataSourceNames, generatedBeanBuildItem).createFlywayProducerBean();
 
-        registerNativeImageResources(resourceProducer, generatedResourceProducer,
-                discoverApplicationMigrations(getMigrationLocations(dataSourceNames)));
+        List<String> applicationMigrations = discoverApplicationMigrations(getMigrationLocations(dataSourceNames));
+        recorder.setApplicationMigrationFiles(applicationMigrations);
 
-        containerListenerProducer.produce(
-                new BeanContainerListenerBuildItem(recorder.setFlywayBuildConfig(flywayBuildConfig)));
+        resourceProducer.produce(new NativeImageResourceBuildItem(applicationMigrations.toArray(new String[0])));
+
+        containerListenerProducer.produce(new BeanContainerListenerBuildItem(recorder.setFlywayBuildConfig(flywayBuildConfig)));
     }
 
     /**
      * Handles all the operations that can be recorded in the RUNTIME_INIT execution time phase
      *
      * @param recorder Used to set the runtime config
      * @param flywayRuntimeConfig The Flyway configuration
-     * @param dataSourceInitializedBuildItem Added this dependency to be sure that Agroal is initialized first
+     * @param jdbcDataSourceBuildItems Added this dependency to be sure that Agroal is initialized first
      */
     @Record(ExecutionTime.RUNTIME_INIT)
     @BuildStep
@@ -120,47 +115,29 @@ ServiceStartBuildItem configureRuntimeProperties(FlywayRecorder recorder,
         return new ServiceStartBuildItem("flyway");
     }
 
-    private void registerNativeImageResources(BuildProducer<NativeImageResourceBuildItem> resource,
-            BuildProducer<GeneratedResourceBuildItem> generatedResourceProducer,
-            List<String> applicationMigrations)
-            throws IOException, URISyntaxException {
-        final List<String> nativeResources = new ArrayList<>();
-        nativeResources.addAll(applicationMigrations);
-        // Store application migration in a generated resource that will be accessed later by the Quarkus-Flyway path scanner
-        String resourcesList = applicationMigrations
-                .stream()
-                .collect(Collectors.joining("\n", "", "\n"));
-        generatedResourceProducer.produce(
-                new GeneratedResourceBuildItem(
-                        QuarkusPathLocationScanner.MIGRATIONS_LIST_FILE,
-                        resourcesList.getBytes(StandardCharsets.UTF_8)));
-        nativeResources.add(QuarkusPathLocationScanner.MIGRATIONS_LIST_FILE);
-        resource.produce(new NativeImageResourceBuildItem(nativeResources.toArray(new String[0])));
-    }
-
     /**
      * Collects the configured migration locations for the default and all named DataSources.
      * <p>
      * A {@link LinkedHashSet} is used to avoid duplications.
      *
-     * @param dataSourceInitializedBuildItem {@link JdbcDataSourceInitializedBuildItem}
-     * @return {@link Collection} of {@link String}s
+     * @return {@link Collection} of {@link String}
      */
     private Collection<String> getMigrationLocations(Collection<String> dataSourceNames) {
         Collection<String> migrationLocations = dataSourceNames.stream()
                 .map(flywayBuildConfig::getConfigForDataSourceName)
                 .flatMap(config -> config.locations.stream())
                 .collect(Collectors.toCollection(LinkedHashSet::new));
+
         if (DataSourceUtil.hasDefault(dataSourceNames)) {
             migrationLocations.addAll(flywayBuildConfig.defaultDataSource.locations);
         }
+
         return migrationLocations;
     }
 
-    private List<String> discoverApplicationMigrations(Collection<String> locations)
-            throws IOException, URISyntaxException {
-        List<String> resources = new ArrayList<>();
+    private List<String> discoverApplicationMigrations(Collection<String> locations) throws IOException, URISyntaxException {
         try {
+            List<String> applicationMigrationResources = new ArrayList<>();
             // Locations can be a comma separated list
             for (String location : locations) {
                 // Strip any 'classpath:' protocol prefixes because they are assumed
@@ -187,11 +164,11 @@ private List<String> discoverApplicationMigrations(Collection<String> locations)
                         applicationMigrations = null;
                     }
                     if (applicationMigrations != null) {
-                        resources.addAll(applicationMigrations);
+                        applicationMigrationResources.addAll(applicationMigrations);
                     }
                 }
             }
-            return resources;
+            return applicationMigrationResources;
         } catch (IOException | URISyntaxException e) {
             throw e;
         }

diff --git a/extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/FlywayRecorder.java b/extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/FlywayRecorder.java
@@ -1,6 +1,7 @@
 package io.quarkus.flyway.runtime;
 
 import java.lang.annotation.Annotation;
+import java.util.List;
 import java.util.Map.Entry;
 
 import javax.enterprise.inject.Default;
@@ -11,10 +12,14 @@
 import io.quarkus.arc.runtime.BeanContainer;
 import io.quarkus.arc.runtime.BeanContainerListener;
 import io.quarkus.flyway.FlywayDataSource;
+import io.quarkus.flyway.runtime.graal.QuarkusPathLocationScanner;
 import io.quarkus.runtime.annotations.Recorder;
 
 @Recorder
 public class FlywayRecorder {
+    public void setApplicationMigrationFiles(List<String> migrationFiles) {
+        QuarkusPathLocationScanner.setApplicationMigrationFiles(migrationFiles);
+    }
 
     public BeanContainerListener setFlywayBuildConfig(FlywayBuildTimeConfig flywayBuildConfig) {
         return beanContainer -> {

diff --git a/...way/runtime/src/main/java/io/quarkus/flyway/runtime/graal/QuarkusPathLocationScanner.java b/...way/runtime/src/main/java/io/quarkus/flyway/runtime/graal/QuarkusPathLocationScanner.java
@@ -1,18 +1,12 @@
 package io.quarkus.flyway.runtime.graal;
 
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Objects;
-import java.util.Set;
-import java.util.stream.Collectors;
 
+import org.flywaydb.core.api.Location;
 import org.flywaydb.core.api.logging.Log;
 import org.flywaydb.core.api.logging.LogFactory;
 import org.flywaydb.core.internal.resource.LoadableResource;
@@ -21,32 +15,46 @@
 
 public final class QuarkusPathLocationScanner implements ResourceAndClassScanner {
     private static final Log LOG = LogFactory.getLog(QuarkusPathLocationScanner.class);
-    /**
-     * File with the migrations list. It is generated dynamically in the Flyway Quarkus Processor
-     */
-    public final static String MIGRATIONS_LIST_FILE = "META-INF/flyway-migrations.txt";
+    private static final String LOCATION_SEPARATOR = "/";
+    private static List<String> applicationMigrationFiles;
+
+    private final Collection<LoadableResource> scannedResources;
+
+    public QuarkusPathLocationScanner(Collection<Location> locations) {
+        this.scannedResources = new ArrayList<>();
+        ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
+
+        for (String migrationFile : applicationMigrationFiles) {
+            if (canHandleMigrationFile(locations, migrationFile)) {
+                LOG.debug("Loading " + migrationFile);
+                scannedResources.add(new ClassPathResource(null, migrationFile, classLoader, StandardCharsets.UTF_8));
+            }
+        }
+
+    }
 
     /**
-     * Returns the migrations loaded into the {@see MIGRATIONS_LIST_FILE}
      *
      * @return The resources that were found.
      */
     @Override
     public Collection<LoadableResource> scanForResources() {
-        ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
-        try (InputStream resource = classLoader.getResourceAsStream(MIGRATIONS_LIST_FILE);
-                BufferedReader reader = new BufferedReader(
-                        new InputStreamReader(Objects.requireNonNull(resource), StandardCharsets.UTF_8))) {
-            List<String> migrations = reader.lines().collect(Collectors.toList());
-            Set<LoadableResource> resources = new HashSet<>();
-            for (String file : migrations) {
-                LOG.debug("Loading " + file);
-                resources.add(new ClassPathResource(null, file, classLoader, StandardCharsets.UTF_8));
+        return scannedResources;
+    }
+
+    private boolean canHandleMigrationFile(Collection<Location> locations, String migrationFile) {
+        for (Location location : locations) {
+            String locationPath = location.getPath();
+            if (!locationPath.endsWith(LOCATION_SEPARATOR)) {
+                locationPath += "/";
+            }
+
+            if (migrationFile.startsWith(locationPath)) {
+                return true;
             }
-            return resources;
-        } catch (IOException e) {
-            throw new IllegalStateException(e);
         }
+
+        return false;
     }
 
     /**
@@ -60,4 +68,8 @@ public Collection<Class<?>> scanForClasses() {
         // Classes are not supported in native mode
         return Collections.emptyList();
     }
+
+    public static void setApplicationMigrationFiles(List<String> applicationMigrationFiles) {
+        QuarkusPathLocationScanner.applicationMigrationFiles = applicationMigrationFiles;
+    }
 }
diff --git a/...ns/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/graal/ScannerSubstitutions.java b/...ns/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/graal/ScannerSubstitutions.java
@@ -36,7 +36,7 @@ public final class ScannerSubstitutions {
     @Substitute
     public ScannerSubstitutions(Class<?> implementedInterface, Collection<Location> locations, ClassLoader classLoader,
             Charset encoding, ResourceNameCache resourceNameCache) {
-        ResourceAndClassScanner quarkusScanner = new QuarkusPathLocationScanner();
+        ResourceAndClassScanner quarkusScanner = new QuarkusPathLocationScanner(locations);
         resources.addAll(quarkusScanner.scanForResources());
         classes.addAll(quarkusScanner.scanForClasses());
     }

diff --git a/integration-tests/flyway/src/main/java/io/quarkus/it/flyway/FlywayFunctionalityResource.java b/integration-tests/flyway/src/main/java/io/quarkus/it/flyway/FlywayFunctionalityResource.java
@@ -13,13 +13,19 @@
 import org.flywaydb.core.Flyway;
 import org.flywaydb.core.api.MigrationVersion;
 
+import io.quarkus.flyway.FlywayDataSource;
+
 @Path("/")
 @Consumes(MediaType.APPLICATION_JSON)
 @Produces(MediaType.APPLICATION_JSON)
 public class FlywayFunctionalityResource {
     @Inject
     Flyway flyway;
 
+    @Inject
+    @FlywayDataSource("second-datasource")
+    Flyway flyway2;
+
     @GET
     @Path("migrate")
     public String doMigrateAuto() {
@@ -29,10 +35,19 @@ public String doMigrateAuto() {
         return version.toString();
     }
 
+    @GET
+    @Path("multiple-flyway-migratation")
+    public String doMigratationOfSecondDataSource() {
+        flyway2.migrate();
+        MigrationVersion version = Objects.requireNonNull(flyway2.info().current().getVersion(),
+                "Version is null! Migration was not applied for second datasource");
+        return version.toString();
+    }
+
     @GET
     @Path("placeholders")
     public Map<String, String> returnPlaceholders() {
         return flyway.getConfiguration().getPlaceholders();
     }
 
-}
+}
diff --git a/integration-tests/flyway/src/main/resources/application.properties b/integration-tests/flyway/src/main/resources/application.properties
@@ -5,8 +5,9 @@ quarkus.log.category."io.quarkus.flyway".level=DEBUG
 quarkus.datasource.db-kind=h2
 quarkus.datasource.username=sa
 quarkus.datasource.password=sa
+
+# default flyway configuration properties
 quarkus.datasource.jdbc.url=jdbc:h2:tcp://localhost/mem:test_quarkus;DB_CLOSE_DELAY=-1
-# Flyway config properties
 quarkus.flyway.connect-retries=10
 quarkus.flyway.schemas=TEST_SCHEMA
 quarkus.flyway.table=flyway_quarkus_history
@@ -15,4 +16,14 @@ quarkus.flyway.sql-migration-prefix=V
 quarkus.flyway.migrate-at-start=true
 quarkus.flyway.placeholders.foo=bar
 
-quarkus.hibernate-orm.database.generation=validate
+quarkus.hibernate-orm.database.generation=validate
+
+# second Agroal config
+quarkus.datasource.second-datasource.db-kind=h2
+quarkus.datasource.second-datasource.jdbc.url=jdbc:h2:tcp://localhost/mem:test_multiple_flyway_datasource;DB_CLOSE_DELAY=-1
+
+# configuration to test multiple flyway datasources
+quarkus.flyway.second-datasource.locations=db/location3
+quarkus.flyway.second-datasource.sql-migration-prefix=V
+quarkus.flyway.second-datasource.migrate-at-start=true
+quarkus.flyway.second-datasource.placeholders.mambo=poa
diff --git a/integration-tests/flyway/src/main/resources/db/location3/V1.0.0__Quarkus.sql b/integration-tests/flyway/src/main/resources/db/location3/V1.0.0__Quarkus.sql
@@ -0,0 +1,7 @@
+CREATE TABLE multiple_flyway_test
+(
+    id   INT,
+    name VARCHAR(255)
+);
+INSERT INTO multiple_flyway_test(id, name)
+VALUES (1, 'Multiple flyway datasources should work seamlessly in JVM and native mode');
diff --git a/integration-tests/flyway/src/test/java/io/quarkus/it/flyway/FlywayFunctionalityTest.java b/integration-tests/flyway/src/test/java/io/quarkus/it/flyway/FlywayFunctionalityTest.java
@@ -18,6 +18,12 @@ public void testFlywayQuarkusFunctionality() {
         when().get("/flyway/migrate").then().body(is("1.0.1"));
     }
 
+    @Test
+    @DisplayName("Migrates a schema correctly using second instance of Flyway")
+    public void testMultipleFlywayQuarkusFunctionality() {
+        when().get("/flyway/multiple-flyway-migratation").then().body(is("1.0.0"));
+    }
+
     @Test
     @DisplayName("Returns current placeholders")
     public void testPlaceholders() {