[DAT-16377] Verify if supports method is implemented in a change to prevent unexpected behaviors

liquibase-cli/src/test/groovy/liquibase/integration/commandline/LiquibaseCommandLineTest.groovy

@@ -430,6 +430,16 @@ Global Options
  supportPropertyEscaping', environment variable:
  'LIQUIBASE_SUPPORT_PROPERTY_ESCAPING')
 
+ --supports-method-validation-levels=PARAM
+ Controls the level of validation performed on the
+ supports method of Change classes. Options are
+ OFF, WARN, FAIL.
+ DEFAULT: WARN
+ (defaults file: 'liquibase.
+ supportsMethodValidationLevels', environment
+ variable:
+ 'LIQUIBASE_SUPPORTS_METHOD_VALIDATION_LEVELS')
+
  --ui-service=PARAM Changes the default UI Service Logger used by
  Liquibase. Options are CONSOLE or LOGGER.
  DEFAULT: CONSOLE

liquibase-standard/src/main/java/liquibase/GlobalConfiguration.java

@@ -51,6 +51,7 @@ public class GlobalConfiguration implements AutoloadedConfigurations {
  public static final ConfigurationDefinition<String> SEARCH_PATH;
 
  public static final ConfigurationDefinition<UIServiceEnum> UI_SERVICE;
+ public static final ConfigurationDefinition<SupportsMethodValidationLevelsEnum> SUPPORTS_METHOD_VALIDATION_LEVELS;
 
  static {
  ConfigurationDefinition.Builder builder = new ConfigurationDefinition.Builder("liquibase");
@@ -242,10 +243,16 @@ public class GlobalConfiguration implements AutoloadedConfigurations {
  .setDescription("Changes the default UI Service Logger used by Liquibase. Options are CONSOLE or LOGGER.")
  .setDefaultValue(UIServiceEnum.CONSOLE)
  .build();
+
+ SUPPORTS_METHOD_VALIDATION_LEVELS = builder.define("supportsMethodValidationLevels", SupportsMethodValidationLevelsEnum.class)
+ .setDescription("Controls the level of validation performed on the supports method of Change classes. Options are OFF, WARN, FAIL.")
+ .setDefaultValue(SupportsMethodValidationLevelsEnum.WARN)
+ .build();
  }
 
  public enum DuplicateFileMode {
  WARN,
  ERROR,
  }
+
 }

liquibase-standard/src/main/java/liquibase/SupportsMethodValidationLevelsEnum.java

@@ -0,0 +1,10 @@
+package liquibase;
+
+/**
+ * Enum to control the level of validation to check if a change's supports method is properly implemented.
+ */
+public enum SupportsMethodValidationLevelsEnum {
+ OFF,
+ WARN,
+ FAIL
+ }

liquibase-standard/src/main/java/liquibase/change/ChangeFactory.java

@@ -1,12 +1,16 @@
 package liquibase.change;
 
 import liquibase.ChecksumVersion;
+import liquibase.GlobalConfiguration;
 import liquibase.Scope;
 import liquibase.database.Database;
 import liquibase.exception.UnexpectedLiquibaseException;
 import liquibase.plugin.AbstractPluginFactory;
 import liquibase.plugin.Plugin;
 import liquibase.servicelocator.ServiceLocator;
+import liquibase.SupportsMethodValidationLevelsEnum;
+import liquibase.util.LiquibaseUtil;
+import lombok.Setter;
 
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
@@ -21,8 +25,15 @@
 public class ChangeFactory extends AbstractPluginFactory<Change>{
 
  private final Map<String, ChangeMetaData> cachedMetadata = new ConcurrentHashMap<>();
+
+ /**
+ * Should the change be checked to see if it supports the current database?
+ */
+ @Setter
  private boolean performSupportsDatabaseValidation = true;
 
+ private static final String SUPPORTS_METHOD_REQUIRED_MESSAGE = "%s class does not implement the 'supports(Database)' method and may incorrectly override other databases changes causing unexpected behavior. Please report this to the Liquibase developers or if you are developing this change please fix it ;)";
+
  private ChangeFactory() {
 
  }
@@ -104,6 +115,9 @@ public Change create(String name) {
  if (plugins.isEmpty()) {
  return null;
  } else if (plugins.size() > 1) {
+ // we only verify supports method if we have more than 1 implementation for this change,
+ // otherwise there is no use in doing that as we will use the only implementation available
+ this.verifySupportsMethodImplementation(plugins);
  Database database = Scope.getCurrentScope().getDatabase();
  if (database != null && performSupportsDatabaseValidation) {
  plugins.removeIf(a -> !a.supports(database));
@@ -120,6 +134,43 @@ public Change create(String name) {
  }
  }
 
+ /**
+ * Verify if the supports method is implemented in the change if it's not part of the default liquibase changes
+ */
+ private void verifySupportsMethodImplementation(Set<Change> plugins) {
+ if (GlobalConfiguration.SUPPORTS_METHOD_VALIDATION_LEVELS.getCurrentValue().equals(SupportsMethodValidationLevelsEnum.OFF)) {
+ return;
+ }
+ //we only verify supports method if this is not part of the default liquibase changes
+ for (Change plugin : plugins) {
+ String packageName = plugin.getClass().getPackage().getName();
+ if (packageName.startsWith("liquibase.change") || packageName.startsWith("com.datical.liquibase.ext")) {
+ continue;
+ }
+
+ try {
+ // if the supports method is not implemented in the plugin show the wraning according to the defined level
+ if (plugin.getClass().getMethod("supports", Database.class).getDeclaringClass().getPackage().getName().startsWith("liquibase.change")) {
+ if (LiquibaseUtil.getBuildVersion().equals(LiquibaseUtil.DEV_VERSION)) {
+ throw new UnexpectedLiquibaseException(String.format(SUPPORTS_METHOD_REQUIRED_MESSAGE, plugin.getClass().getName()));
+ }
+ switch (GlobalConfiguration.SUPPORTS_METHOD_VALIDATION_LEVELS.getCurrentValue()) {
+ case WARN:
+ Scope.getCurrentScope().getLog(getClass()).warning(String.format(SUPPORTS_METHOD_REQUIRED_MESSAGE, plugin.getClass().getName()));
+ plugins.remove(plugin);
+ break;
+ case FAIL:
+ throw new UnexpectedLiquibaseException(String.format(SUPPORTS_METHOD_REQUIRED_MESSAGE, plugin.getClass().getName()));
+ default:
+ break;
+ }
+ }
+ } catch (NoSuchMethodException e) {
+ throw new UnexpectedLiquibaseException(String.format(SUPPORTS_METHOD_REQUIRED_MESSAGE, plugin.getClass().getName()));
+ }
+ }
+ }
+
  public Map<String, Object> getParameters(Change change) {
  Map<String, Object> returnMap = new HashMap<>();
  ChangeMetaData changeMetaData = getChangeMetaData(change);
@@ -141,14 +192,4 @@ public static ChangeFactory getInstance() {
  return Scope.getCurrentScope().getSingleton(ChangeFactory.class);
  }
 
- /**
- * Should the change be checked to see if it supports
- * the current database?
- * Default is true
- *
- * @param performSupportsDatabaseValidation
- */
- public void setPerformSupportsDatabaseValidation(boolean performSupportsDatabaseValidation) {
- this.performSupportsDatabaseValidation = performSupportsDatabaseValidation;
- }
 }

liquibase-standard/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java

@@ -104,7 +104,7 @@ private void warnForMismatchedXsdVersion(String systemId) {
  boolean found = versionMatcher.find();
  if (found) {
  String buildVersion = LiquibaseUtil.getBuildVersion();
- if (!buildVersion.equals("DEV")) {
+ if (!buildVersion.equals(LiquibaseUtil.DEV_VERSION)) {
  String xsdVersion = versionMatcher.group("version");
  if (!buildVersion.startsWith(xsdVersion)) {
  hasWarnedAboutMismatchedXsdVersion = true;

liquibase-standard/src/main/java/liquibase/util/LiquibaseUtil.java

@@ -12,10 +12,12 @@
 
 public class LiquibaseUtil {
 
+ public static final String DEV_VERSION = "DEV";
+
  private static Properties liquibaseBuildProperties;
 
  /**
- * Return the configured build.version. Will always be "DEV" for non-release builds.
+ * Return the configured build.version. Will always be DEV_VERSION for non-release builds.
  */
  public static String getBuildVersion() {
  return getBuildInfo("build.version");
@@ -26,7 +28,7 @@ public static String getBuildVersion() {
  */
  public static String getBuildVersionInfo() {
  String version = getBuildInfo("build.version");
- if (version.equals("DEV")) {
+ if (version.equals(DEV_VERSION)) {
  final String buildCommit = getBuildInfo("build.commit");
  if (buildCommit.equals("unknown")) {
  version = "[local build]";

liquibase-standard/src/test/groovy/liquibase/parser/core/xml/XMLChangeLogSAXParserTest.groovy

@@ -13,6 +13,7 @@ import liquibase.exception.ChangeLogParseException
 import liquibase.exception.LiquibaseException
 import liquibase.sdk.resource.MockResourceAccessor
 import liquibase.test.JUnitResourceAccessor
+import liquibase.util.LiquibaseUtil
 import spock.lang.Specification
 import spock.lang.Unroll
 
@@ -173,13 +174,13 @@ class XMLChangeLogSAXParserTest extends Specification {
  XMLChangeLogSAXParser.computeSchemaVersion(buildVersion) == expected
 
  where:
- buildVersion | expected
- "DEV"  | "latest"
- "4.11.0" | "4.11"
- "4.11.1" | "4.11"
- "4" | "latest" //weird versions go to latest
- "" | "latest" //weird versions go to latest
- null | "latest" //weird versions go to latest
+ buildVersion  | expected
+ LiquibaseUtil.DEV_VERSION | "latest"
+ "4.11.0"  | "4.11"
+ "4.11.1"  | "4.11"
+ "4"  | "latest" //weird versions go to latest
+ ""  | "latest" //weird versions go to latest
+ null  | "latest" //weird versions go to latest
  }
 
 }

liquibase-standard/src/test/groovy/liquibase/util/LiquibaseUtilTest.groovy

@@ -35,13 +35,13 @@ class LiquibaseUtilTest extends Specification {
  LiquibaseUtil.getBuildVersion() == version
 
  where:
- version | localBuild | proData | expected
- "1.2.3" | false | true | "1.2.3"
- "1.2.3" | false | false | "1.2.3"
- "DEV"  | false | true | "[Core: liquibase/liquibase/test-branch/524/7904bb/2021-09-17 14:06+0000, Pro: other-branch/58/e5195b/2021-09-17T14:04:58Z]"
- "DEV"  | false | false | "[Core: liquibase/liquibase/test-branch/524/7904bb/2021-09-17 14:06+0000]"
- "DEV"  | true | true | "[local build]"
- "DEV"  | true | false | "[local build]"
+ version  | localBuild | proData | expected
+ "1.2.3"  | false | true | "1.2.3"
+ "1.2.3"  | false | false | "1.2.3"
+ LiquibaseUtil.DEV_VERSION | false | true | "[Core: liquibase/liquibase/test-branch/524/7904bb/2021-09-17 14:06+0000, Pro: other-branch/58/e5195b/2021-09-17T14:04:58Z]"
+ LiquibaseUtil.DEV_VERSION | false | false | "[Core: liquibase/liquibase/test-branch/524/7904bb/2021-09-17 14:06+0000]"
+ LiquibaseUtil.DEV_VERSION | true | true | "[local build]"
+ LiquibaseUtil.DEV_VERSION | true | false | "[local build]"
  }
 
  def "getBuildVersionInfo actuallyRead"() {