HBX-3166: Same usage order of reveng files and strategy for all tools

ant/src/main/java/org/hibernate/tool/ant/JDBCConfigurationTask.java

@@ -78,12 +78,8 @@ private RevengStrategy createReverseEngineeringStrategy() {
 
 		RevengStrategy strategy = 
 				RevengStrategyFactory.createReverseEngineeringStrategy(
-						null, revengFileList);
-
-		if(reverseEngineeringStrategyClass!=null) {
-			strategy = loadreverseEngineeringStrategy(reverseEngineeringStrategyClass, strategy);			
-		}
-
+                        reverseEngineeringStrategyClass, revengFileList);
+
 		RevengSettings qqsettings = 
 			new RevengSettings(strategy).setDefaultPackageName(packageName)
 			.setDetectManyToMany( detectManyToMany )
@@ -124,31 +120,6 @@ public void setDetectOptimisticLock(boolean b) {
 		detectOptimisticLock = b;
 	}
 
-    private RevengStrategy loadreverseEngineeringStrategy(final String className, RevengStrategy delegate)
-    throws BuildException {
-        try {
-            Class<?> clazz = ReflectionUtil.classForName(className);			
-			Constructor<?> constructor = clazz.getConstructor(new Class[] { RevengStrategy.class });
-            return (RevengStrategy) constructor.newInstance(new Object[] { delegate }); 
-        } 
-        catch (NoSuchMethodException e) {
-			try {
-				getProject().log("Could not find public " + className + "(ReverseEngineeringStrategy delegate) constructor on ReverseEngineeringStrategy. Trying no-arg version.",Project.MSG_VERBOSE);			
-				Class<?> clazz = ReflectionUtil.classForName(className);	
-				Constructor<?> constructor = clazz.getConstructor(new Class[] {});
-				RevengStrategy rev = (RevengStrategy) constructor.newInstance();
-				getProject().log("Using non-delegating strategy, thus packagename and revengfile will be ignored.", Project.MSG_INFO);
-				return rev;
-			} 
-			catch (Exception eq) {
-				throw new BuildException("Could not create or find " + className + " with default no-arg constructor", eq);
-			}
-		} 
-        catch (Exception e) {
-			throw new BuildException("Could not create or find " + className + " with one argument delegate constructor", e);
-		} 
-    }
-
 	private Map<String, Object> loadCfgXmlFile() {
 		return new ConfigLoader(new BootstrapServiceRegistryBuilder().build())
 				.loadConfigXmlFile(getConfigurationFile())

orm/src/main/java/org/hibernate/tool/api/reveng/RevengStrategyFactory.java

@@ -24,46 +24,88 @@
 import org.hibernate.tool.internal.reveng.strategy.DefaultStrategy;
 import org.hibernate.tool.internal.reveng.strategy.OverrideRepository;
 import org.hibernate.tool.util.ReflectionUtil;
+import org.jboss.logging.Logger;
 
 public class RevengStrategyFactory {
-
-	private static final String DEFAULT_REVERSE_ENGINEERING_STRATEGY_CLASS_NAME = 
-			DefaultStrategy.class.getName();
-
+    final private static Logger log = Logger.getLogger( RevengStrategyFactory.class );
+
+    private static RevengStrategy createDefaultStrategyInstance() {
+        return new DefaultStrategy();
+    }
+
+    private static RuntimeException createExceptionBecauseOfMissingConstructors(Class<?> revengClass) {
+        return new RuntimeException("A strategy of class '" + revengClass.getName() + "' could not be created. " +
+                "No matching constructor found: '" + revengClass.getSimpleName() + "()' or " +
+                "'" + revengClass.getSimpleName() + "(" + RevengStrategy.class.getName() + ")'");
+    }
+
 	public static RevengStrategy createReverseEngineeringStrategy(
-			String reverseEngineeringClassName) {
-		RevengStrategy result = null;
-		try {
-			Class<?> reverseEngineeringClass = 
-					ReflectionUtil.classForName(
-							reverseEngineeringClassName == null ? 
-									DEFAULT_REVERSE_ENGINEERING_STRATEGY_CLASS_NAME : 
-									reverseEngineeringClassName);
-			Constructor<?> reverseEngineeringConstructor = reverseEngineeringClass.getConstructor(new Class[] {});
-			result = (RevengStrategy)reverseEngineeringConstructor.newInstance();
-		} catch (ClassNotFoundException | IllegalAccessException | InstantiationException | NoSuchMethodException | SecurityException | IllegalArgumentException | InvocationTargetException exception) {
-			throw new RuntimeException("An exporter of class '" + reverseEngineeringClassName + "' could not be created", exception);
-		}
-		return result;
+			String revengClassName, RevengStrategy delegate) {
+        if (revengClassName == null) {
+            return delegate == null ? createDefaultStrategyInstance() : delegate;
+        }
+
+        try {
+            Class<?> revengClass = ReflectionUtil.classForName(revengClassName);
+
+            if (delegate == null) {
+                try {
+                    try {
+                        Constructor<?> revengConstructor = revengClass.getConstructor();
+                        return (RevengStrategy) revengConstructor.newInstance();
+                    } catch (NoSuchMethodException e1) {
+                        try {
+                            Constructor<?> revengConstructor = revengClass.getConstructor(RevengStrategy.class);
+                            return (RevengStrategy) revengConstructor.newInstance(createDefaultStrategyInstance());
+                        } catch (NoSuchMethodException e2) {
+                            throw createExceptionBecauseOfMissingConstructors(revengClass);
+                        }
+                    }
+                } catch (IllegalAccessException | InstantiationException | InvocationTargetException | IllegalArgumentException e) {
+                    throw new RuntimeException("A strategy of class '" + revengClassName + "' could not be created", e);
+                }
+            }
+
+            try {
+                try {
+                    Constructor<?> revengConstructor = revengClass.getConstructor(RevengStrategy.class);
+                    return (RevengStrategy) revengConstructor.newInstance(delegate);
+                } catch (NoSuchMethodException e1) {
+                    log.warn("Delegating strategy given, but no matching constructor is available on class '" +
+                            revengClassName + "'. The delegating strategy will be ignored!");
+                    try {
+                        Constructor<?> revengConstructor = revengClass.getConstructor();
+                        return (RevengStrategy) revengConstructor.newInstance();
+                    } catch (NoSuchMethodException e2) {
+                        throw createExceptionBecauseOfMissingConstructors(revengClass);
+                    }
+                }
+            } catch (IllegalAccessException | InstantiationException | InvocationTargetException | IllegalArgumentException e) {
+                throw new RuntimeException("A strategy of class '" + revengClassName + "' could not be created", e);
+            }
+        } catch (ClassNotFoundException e) {
+            throw new RuntimeException("A strategy of class '" + revengClassName + "' could not be created", e);
+        }
 	}
 
-	public static RevengStrategy createReverseEngineeringStrategy(
-			String reverseEngineeringClassName,
-			File[] revengFiles) {
-		RevengStrategy result = 
-				createReverseEngineeringStrategy(reverseEngineeringClassName);
-		if (revengFiles != null && revengFiles.length > 0) {
-			OverrideRepository overrideRepository = new OverrideRepository();
-			for (File file : revengFiles) {
-				overrideRepository.addFile(file);
-			}
-			result = overrideRepository.getReverseEngineeringStrategy(result);
-		}
-		return result;
+	public static RevengStrategy createReverseEngineeringStrategy( String revengClassName, File[] revengFiles) {
+        RevengStrategy result = null;
+        if (revengFiles != null && revengFiles.length > 0) {
+            OverrideRepository overrideRepository = new OverrideRepository();
+            for (File file : revengFiles) {
+                overrideRepository.addFile(file);
+            }
+            result = overrideRepository.getReverseEngineeringStrategy(createDefaultStrategyInstance());
+        }
+
+		return createReverseEngineeringStrategy(revengClassName, result);
 	}
-
+
+    public static RevengStrategy createReverseEngineeringStrategy(String revengClassName) {
+        return createReverseEngineeringStrategy(revengClassName, (RevengStrategy) null);
+    }
+
 	public static RevengStrategy createReverseEngineeringStrategy() {
-		return createReverseEngineeringStrategy(null);
+		return createDefaultStrategyInstance();
 	}
-
 }

orm/src/main/java/org/hibernate/tool/internal/reveng/strategy/OverrideRepository.java

@@ -602,6 +602,11 @@ private Map<String, MetaAttribute> toMetaAttributes(MultiValuedMap<String, Simpl
 		return result;
  	}
 
+    /**
+     * @deprecated Use {@link #getReverseEngineeringStrategy(RevengStrategy)}
+     *     with {@code delegate=null} to explicitly ignore the delegate.
+     */
+     @Deprecated
 	public RevengStrategy getReverseEngineeringStrategy() {
 		return getReverseEngineeringStrategy(null);
 	}

test/common/src/main/java/org/hibernate/tool/ant/Cfg2HbmWithInvalidReverseNamingStrategy/TestCase.java

@@ -74,7 +74,7 @@ public void testCfg2HbmWithInvalidReverseNamingStrategy() {
 
 			assertTrue(e
 					.getMessage()
-					.contains("Could not create or find invalid.Strategy"));
+					.contains("A strategy of class 'invalid.Strategy' could not be created"));
 
 		}
 

test/common/src/main/java/org/hibernate/tool/jdbc2cfg/OverrideBinder/TestCase.java

@@ -401,7 +401,7 @@ public void testColumnExclude() {
 		OverrideRepository or = new OverrideRepository();
 		or.addResource(OVERRIDETEST_REVENG_XML);
 
-		RevengStrategy reverseEngineeringStrategy = or.getReverseEngineeringStrategy();
+		RevengStrategy reverseEngineeringStrategy = or.getReverseEngineeringStrategy(null);
 
 		assertFalse(reverseEngineeringStrategy.excludeColumn(TableIdentifier.create(null, null, "EXCOLUMNS"), "blah"));
 		assertFalse(reverseEngineeringStrategy.excludeColumn(TableIdentifier.create(null, null, "EXCOLUMNS"), "NAME"));