From ba24c3c4188213956e1040679dfb2cb5b913b3b3 Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Wed, 11 Sep 2019 07:30:25 +0100 Subject: [PATCH] Fix #2482: Warn about invalid Java migrations --- .../main/java/org/flywaydb/core/Flyway.java | 9 ++++-- .../core/internal/clazz/ClassProvider.java | 7 ++--- .../internal/clazz/NoopClassProvider.java | 2 +- .../resolver/CompositeMigrationResolver.java | 3 +- .../java/ScanningJavaMigrationResolver.java | 6 ++-- .../core/internal/scanner/Scanner.java | 29 +++++++------------ .../scanner/android/AndroidScanner.java | 12 ++++---- .../scanner/classpath/ClassPathScanner.java | 15 ++++++---- .../classpath/ResourceAndClassScanner.java | 6 ++-- .../core/internal/util/ClassUtils.java | 29 ++++++++++++++----- 10 files changed, 66 insertions(+), 52 deletions(-) diff --git a/flyway-core/src/main/java/org/flywaydb/core/Flyway.java b/flyway-core/src/main/java/org/flywaydb/core/Flyway.java index 0be95ea3b8..efd7872506 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/Flyway.java +++ b/flyway-core/src/main/java/org/flywaydb/core/Flyway.java @@ -23,6 +23,7 @@ import org.flywaydb.core.api.configuration.FluentConfiguration; import org.flywaydb.core.api.logging.Log; import org.flywaydb.core.api.logging.LogFactory; +import org.flywaydb.core.api.migration.JavaMigration; import org.flywaydb.core.api.resolver.MigrationResolver; import org.flywaydb.core.internal.callback.CallbackExecutor; import org.flywaydb.core.internal.callback.DefaultCallbackExecutor; @@ -371,7 +372,7 @@ public Void execute(MigrationResolver migrationResolver, * @return A new, fully configured, MigrationResolver instance. */ private MigrationResolver createMigrationResolver(ResourceProvider resourceProvider, - ClassProvider classProvider, + ClassProvider classProvider, SqlScriptExecutorFactory sqlScriptExecutorFactory, SqlScriptFactory sqlScriptFactory) { return new CompositeMigrationResolver(resourceProvider, classProvider, configuration, @@ -410,12 +411,14 @@ private MigrationResolver createMigrationResolver(ResourceProvider resourceProvi final ResourceProvider resourceProvider; - ClassProvider classProvider; + ClassProvider classProvider; if (!scannerRequired && configuration.isSkipDefaultResolvers() && configuration.isSkipDefaultCallbacks()) { resourceProvider = NoopResourceProvider.INSTANCE; + //noinspection unchecked classProvider = NoopClassProvider.INSTANCE; } else { - Scanner scanner = new Scanner( + Scanner scanner = new Scanner<>( + JavaMigration.class, Arrays.asList(configuration.getLocations()), configuration.getClassLoader(), configuration.getEncoding() diff --git a/flyway-core/src/main/java/org/flywaydb/core/internal/clazz/ClassProvider.java b/flyway-core/src/main/java/org/flywaydb/core/internal/clazz/ClassProvider.java index 1dbf3a885b..f642f35d7c 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/internal/clazz/ClassProvider.java +++ b/flyway-core/src/main/java/org/flywaydb/core/internal/clazz/ClassProvider.java @@ -20,12 +20,11 @@ /** * A facility to obtain classes. */ -public interface ClassProvider { +public interface ClassProvider { /** - * Retrieve all classes which implement this interface. + * Retrieve all classes which implement the specified interface. * - * @param implementedInterface The interface the matching classes should implement. * @return The non-abstract classes that were found. */ - Collection> getClasses(Class implementedInterface); + Collection> getClasses(); } \ No newline at end of file diff --git a/flyway-core/src/main/java/org/flywaydb/core/internal/clazz/NoopClassProvider.java b/flyway-core/src/main/java/org/flywaydb/core/internal/clazz/NoopClassProvider.java index d80d1684b0..4f391879dd 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/internal/clazz/NoopClassProvider.java +++ b/flyway-core/src/main/java/org/flywaydb/core/internal/clazz/NoopClassProvider.java @@ -25,7 +25,7 @@ public enum NoopClassProvider implements ClassProvider { INSTANCE; @Override - public Collection> getClasses(Class implementedInterface) { + public Collection> getClasses() { return Collections.emptyList(); } } \ No newline at end of file diff --git a/flyway-core/src/main/java/org/flywaydb/core/internal/resolver/CompositeMigrationResolver.java b/flyway-core/src/main/java/org/flywaydb/core/internal/resolver/CompositeMigrationResolver.java index 142a90e6de..ed0084aaf1 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/internal/resolver/CompositeMigrationResolver.java +++ b/flyway-core/src/main/java/org/flywaydb/core/internal/resolver/CompositeMigrationResolver.java @@ -17,6 +17,7 @@ import org.flywaydb.core.api.FlywayException; import org.flywaydb.core.api.configuration.Configuration; +import org.flywaydb.core.api.migration.JavaMigration; import org.flywaydb.core.api.resolver.Context; import org.flywaydb.core.api.resolver.MigrationResolver; import org.flywaydb.core.api.resolver.ResolvedMigration; @@ -62,7 +63,7 @@ public class CompositeMigrationResolver implements MigrationResolver { * @param customMigrationResolvers Custom Migration Resolvers. */ public CompositeMigrationResolver(ResourceProvider resourceProvider, - ClassProvider classProvider, + ClassProvider classProvider, Configuration configuration, SqlScriptExecutorFactory sqlScriptExecutorFactory, SqlScriptFactory sqlScriptFactory, diff --git a/flyway-core/src/main/java/org/flywaydb/core/internal/resolver/java/ScanningJavaMigrationResolver.java b/flyway-core/src/main/java/org/flywaydb/core/internal/resolver/java/ScanningJavaMigrationResolver.java index c9450489a0..0acd37a524 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/internal/resolver/java/ScanningJavaMigrationResolver.java +++ b/flyway-core/src/main/java/org/flywaydb/core/internal/resolver/java/ScanningJavaMigrationResolver.java @@ -36,7 +36,7 @@ public class ScanningJavaMigrationResolver implements MigrationResolver { /** * The Scanner to use. */ - private final ClassProvider classProvider; + private final ClassProvider classProvider; /** * The configuration to inject (if necessary) in the migration classes. @@ -49,7 +49,7 @@ public class ScanningJavaMigrationResolver implements MigrationResolver { * @param classProvider The class provider. * @param configuration The configuration to inject (if necessary) in the migration classes. */ - public ScanningJavaMigrationResolver(ClassProvider classProvider, Configuration configuration) { + public ScanningJavaMigrationResolver(ClassProvider classProvider, Configuration configuration) { this.classProvider = classProvider; this.configuration = configuration; } @@ -58,7 +58,7 @@ public ScanningJavaMigrationResolver(ClassProvider classProvider, Configuration public List resolveMigrations(Context context) { List migrations = new ArrayList<>(); - for (Class clazz : classProvider.getClasses(JavaMigration.class)) { + for (Class clazz : classProvider.getClasses()) { JavaMigration javaMigration = ClassUtils.instantiate(clazz.getName(), configuration.getClassLoader()); migrations.add(new ResolvedJavaMigration(javaMigration)); } diff --git a/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/Scanner.java b/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/Scanner.java index fa4ff64a55..f0397bc3fd 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/Scanner.java +++ b/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/Scanner.java @@ -31,18 +31,19 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; /** * Scanner for Resources and Classes. */ -public class Scanner implements ResourceProvider, ClassProvider { +public class Scanner implements ResourceProvider, ClassProvider { private static final Log LOG = LogFactory.getLog(Scanner.class); private final List resources = new ArrayList<>(); - private final List> classes = new ArrayList<>(); + private final List> classes = new ArrayList<>(); - public Scanner(Collection locations, ClassLoader classLoader, Charset encoding + public Scanner(Class implementedInterface, Collection locations, ClassLoader classLoader, Charset encoding @@ -59,9 +60,9 @@ public Scanner(Collection locations, ClassLoader classLoader, Charset if (location.isFileSystem()) { resources.addAll(fileSystemScanner.scanForResources(location)); } else { - ResourceAndClassScanner resourceAndClassScanner = android - ? new AndroidScanner(classLoader, encoding, location) - : new ClassPathScanner(classLoader, encoding, location); + ResourceAndClassScanner resourceAndClassScanner = android + ? new AndroidScanner<>(implementedInterface, classLoader, encoding, location) + : new ClassPathScanner<>(implementedInterface, classLoader, encoding, location); resources.addAll(resourceAndClassScanner.scanForResources()); classes.addAll(resourceAndClassScanner.scanForClasses()); } @@ -100,22 +101,12 @@ public Collection getResources(String prefix, String... suffix } /** - * Scans the classpath for concrete classes under the specified package implementing this interface. + * Scans the classpath for concrete classes under the specified package implementing the specified interface. * Non-instantiable abstract classes are filtered out. * - * @param implementedInterface The interface the matching classes should implement. * @return The non-abstract classes that were found. */ - public Collection> getClasses(Class implementedInterface) { - List> result = new ArrayList<>(); - for (Class clazz : classes) { - if (!implementedInterface.isAssignableFrom(clazz)) { - continue; - } - - //noinspection unchecked - result.add((Class) clazz); - } - return result; + public Collection> getClasses() { + return Collections.unmodifiableCollection(classes); } } \ No newline at end of file diff --git a/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/android/AndroidScanner.java b/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/android/AndroidScanner.java index 50b932e526..8db1c5f4bb 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/android/AndroidScanner.java +++ b/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/android/AndroidScanner.java @@ -37,16 +37,18 @@ /** * Class & resource scanner for Android. */ -public class AndroidScanner implements ResourceAndClassScanner { +public class AndroidScanner implements ResourceAndClassScanner { private static final Log LOG = LogFactory.getLog(AndroidScanner.class); private final Context context; + private final Class implementedInterface; private final ClassLoader clazzLoader; private final Charset encoding; private final Location location; - public AndroidScanner(ClassLoader clazzLoader, Charset encoding, Location location) { + public AndroidScanner(Class implementedInterface, ClassLoader clazzLoader, Charset encoding, Location location) { + this.implementedInterface = implementedInterface; this.clazzLoader = clazzLoader; this.encoding = encoding; this.location = location; @@ -74,10 +76,10 @@ public Collection scanForResources() { } @Override - public Collection> scanForClasses() { + public Collection> scanForClasses() { String pkg = location.getPath().replace("/", "."); - List> classes = new ArrayList<>(); + List> classes = new ArrayList<>(); String sourceDir = context.getApplicationInfo().sourceDir; DexFile dex = null; try { @@ -86,7 +88,7 @@ public Collection> scanForClasses() { while (entries.hasMoreElements()) { String className = entries.nextElement(); if (className.startsWith(pkg)) { - Class clazz = ClassUtils.loadClass(className, clazzLoader); + Class clazz = ClassUtils.loadClass(implementedInterface, className, clazzLoader); if (clazz != null) { classes.add(clazz); } diff --git a/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/classpath/ClassPathScanner.java b/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/classpath/ClassPathScanner.java index 61b67be9bb..f7578a3fa7 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/classpath/ClassPathScanner.java +++ b/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/classpath/ClassPathScanner.java @@ -39,9 +39,10 @@ /** * ClassPath scanner. */ -public class ClassPathScanner implements ResourceAndClassScanner { +public class ClassPathScanner implements ResourceAndClassScanner { private static final Log LOG = LogFactory.getLog(ClassPathScanner.class); + private final Class implementedInterface; /** * The ClassLoader for loading migrations on the classpath. */ @@ -70,7 +71,8 @@ public class ClassPathScanner implements ResourceAndClassScanner { * * @param classLoader The ClassLoader for loading migrations on the classpath. */ - public ClassPathScanner(ClassLoader classLoader, Charset encoding, Location location) { + public ClassPathScanner(Class implementedInterface, ClassLoader classLoader, Charset encoding, Location location) { + this.implementedInterface = implementedInterface; this.classLoader = classLoader; this.location = location; @@ -87,14 +89,17 @@ public Collection scanForResources() { } @Override - public Collection> scanForClasses() { + public Collection> scanForClasses() { LOG.debug("Scanning for classes at " + location); - List> classes = new ArrayList<>(); + List> classes = new ArrayList<>(); for (LoadableResource resource : resources) { if (resource.getAbsolutePath().endsWith(".class")) { - Class clazz = ClassUtils.loadClass(toClassName(resource.getAbsolutePath()), classLoader); + Class clazz = ClassUtils.loadClass( + implementedInterface, + toClassName(resource.getAbsolutePath()), + classLoader); if (clazz != null) { classes.add(clazz); } diff --git a/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/classpath/ResourceAndClassScanner.java b/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/classpath/ResourceAndClassScanner.java index 2eca2ba5fa..1c1b97715d 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/classpath/ResourceAndClassScanner.java +++ b/flyway-core/src/main/java/org/flywaydb/core/internal/scanner/classpath/ResourceAndClassScanner.java @@ -22,7 +22,7 @@ /** * Scanner for both resources and classes. */ -public interface ResourceAndClassScanner { +public interface ResourceAndClassScanner { /** * Scans the classpath for resources under the configured location. * @@ -31,10 +31,10 @@ public interface ResourceAndClassScanner { Collection scanForResources(); /** - * Scans the classpath for concrete classes under the specified package implementing this interface. + * Scans the classpath for concrete classes under the specified package implementing the specified interface. * Non-instantiable abstract classes are filtered out. * * @return The non-abstract classes that were found. */ - Collection> scanForClasses(); + Collection> scanForClasses(); } \ No newline at end of file diff --git a/flyway-core/src/main/java/org/flywaydb/core/internal/util/ClassUtils.java b/flyway-core/src/main/java/org/flywaydb/core/internal/util/ClassUtils.java index 8b977bfeb9..d82f3e7b50 100644 --- a/flyway-core/src/main/java/org/flywaydb/core/internal/util/ClassUtils.java +++ b/flyway-core/src/main/java/org/flywaydb/core/internal/util/ClassUtils.java @@ -117,13 +117,19 @@ public static boolean isPresent(String className, ClassLoader classLoader) { /** * Loads the class with this name using the class loader. * - * @param className The name of the class to load. - * @param classLoader The ClassLoader to use. + * @param implementedInterface The interface the class is expected to implement. + * @param className The name of the class to load. + * @param classLoader The ClassLoader to use. * @return the newly loaded class or {@code null} if it could not be loaded. */ - public static Class loadClass(String className, ClassLoader classLoader) { + public static Class loadClass(Class implementedInterface, String className, ClassLoader classLoader) { try { Class clazz = classLoader.loadClass(className); + + if (!implementedInterface.isAssignableFrom(clazz)) { + return null; + } + if (Modifier.isAbstract(clazz.getModifiers()) || clazz.isEnum() || clazz.isAnonymousClass()) { LOG.debug("Skipping non-instantiable class: " + className); return null; @@ -131,17 +137,24 @@ public static Class loadClass(String className, ClassLoader classLoader) { clazz.getDeclaredConstructor().newInstance(); LOG.debug("Found class: " + className); - return clazz; + //noinspection unchecked + return (Class) clazz; } catch (Throwable e) { Throwable rootCause = ExceptionUtils.getRootCause(e); - LOG.debug("Skipping " + className + " (" + e.getClass().getSimpleName() + "): " + e.getMessage() - + (rootCause == e ? "" : - " caused by " + rootCause.getClass().getSimpleName() + ": " + rootCause.getMessage() - + " at " + ExceptionUtils.getThrowLocation(rootCause))); + LOG.warn("Skipping " + className + ": " + formatThrowable(e) + ( + rootCause == e + ? "" + : " caused by " + formatThrowable(rootCause) + + " at " + ExceptionUtils.getThrowLocation(rootCause) + )); return null; } } + private static String formatThrowable(Throwable e) { + return "(" + e.getClass().getSimpleName() + ": " + e.getMessage() + ")"; + } + /** * Retrieves the physical location on disk of this class. *