HBX-3187: Improve the implementation of PrimaryKeyProcessor

Signed-off-by: Koen Aers <koen.aers@gmail.com>

Author: koentsje

orm/src/main/java/org/hibernate/tool/internal/reveng/reader/PrimaryKeyProcessor.java

@@ -1,11 +1,6 @@
 package org.hibernate.tool.internal.reveng.reader;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 import org.hibernate.JDBCException;
 import org.hibernate.mapping.Column;
@@ -40,30 +35,25 @@ public static void processPrimaryKey(
 			while (primaryKeyIterator.hasNext() ) {
 				primaryKeyRs = primaryKeyIterator.next();
 
-				/*String ownCatalog = primaryKeyRs.getString("TABLE_CAT");
-				 String ownSchema = primaryKeyRs.getString("TABLE_SCHEM");
-				 String ownTable = primaryKeyRs.getString("TABLE_NAME");*/
-				
 				String columnName = (String) primaryKeyRs.get("COLUMN_NAME");
-				short seq = ((Short)primaryKeyRs.get("KEY_SEQ")).shortValue();
+				short seq = (Short) primaryKeyRs.get("KEY_SEQ");
 				String name = (String) primaryKeyRs.get("PK_NAME");
 
 				if(key==null) {
 					key = new PrimaryKey(table);
 					key.setName(name);
-					key.setTable(table);
 					if(table.getPrimaryKey()!=null) {
 						throw new RuntimeException(table + " already has a primary key!"); //TODO: ignore ?
 					}
 					table.setPrimaryKey(key);
 				} 
 				else {
-					if(!(name==key.getName() ) && name!=null && !name.equals(key.getName() ) ) {
+					if(!(Objects.equals(name, key.getName())) && name!=null && !name.equals(key.getName() ) ) {
 						throw new RuntimeException("Duplicate names found for primarykey. Existing name: " + key.getName() + " JDBC name: " + name + " on table " + table);
 					}	      		
 				}
 
-				columns.add(new Object[] { Short.valueOf(seq), columnName});
+				columns.add(new Object[] {seq, columnName});
 			}
 		} finally {
 			if (primaryKeyIterator!=null) {
@@ -75,28 +65,22 @@ public static void processPrimaryKey(
 			}
 		}
 
-	      // sort the columns accoring to the key_seq.
-	      Collections.sort(columns,new Comparator<Object[]>() {
-			public int compare(Object[] o1, Object[] o2) {
-				Short left = (Short)o1[0];
-				Short right = (Short)o2[0];
-				return left.compareTo(right);
-			}
-	      });
+	      columns.sort((o1, o2) -> {
+              Short left = (Short) o1[0];
+              Short right = (Short) o2[0];
+              return left.compareTo(right);
+          });
 
 	      List<String> t = new ArrayList<String>(columns.size());
-	      Iterator<?> cols = columns.iterator();
-	      while (cols.hasNext() ) {
-			Object[] element = (Object[]) cols.next();
-			t.add((String)element[1]);
-	      }
+        for (Object[] element : columns) {
+            t.add((String) element[1]);
+        }
 
 	      if(key==null) {
 	      	log.warn("The JDBC driver didn't report any primary key columns in " + table.getName() + ". Asking rev.eng. strategy" );
 	      	List<String> userPrimaryKey = RevengUtils.getPrimaryKeyInfoInRevengStrategy(revengStrategy, table, defaultCatalog, defaultSchema);	      	if(userPrimaryKey!=null && !userPrimaryKey.isEmpty()) {
 	      		key = new PrimaryKey(table);
 	      		key.setName(new Alias(15, "PK").toAliasString( table.getName()));
-	      		key.setTable(table);
 	      		if(table.getPrimaryKey()!=null) {
 	      			throw new RuntimeException(table + " already has a primary key!"); //TODO: ignore ?
 	      		}
@@ -131,13 +115,11 @@ public int compare(Object[] o1, Object[] o2) {
 	      }
 
 	      if(key!=null) {
-	    	  cols = t.iterator();
-	    	  while (cols.hasNext() ) {
-	    		  String name = (String) cols.next();
-	    		  // should get column from table if it already exists!
-	    		  Column col = getColumn(metaDataDialect, table, name);
-	    		  key.addColumn(col);
-	    	  }
+              for (String name : t) {
+                  // should get column from table if it already exists!
+                  Column col = getColumn(metaDataDialect, table, name);
+                  key.addColumn(col);
+              }
 	    	  log.debug("primary key for " + table + " -> "  + key);
 	      } 
 

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

@@ -29,7 +29,7 @@ public abstract class AbstractStrategy implements RevengStrategy {
 
 	static final private Logger log = Logger.getLogger(AbstractStrategy.class);
 
-	private static Set<String> AUTO_OPTIMISTICLOCK_COLUMNS;
+	private static final Set<String> AUTO_OPTIMISTICLOCK_COLUMNS;
 
 	private RevengSettings settings = new RevengSettings(this);
 
@@ -145,7 +145,7 @@ public String tableToClassName(TableIdentifier tableIdentifier) {
 		String pkgName = settings.getDefaultPackageName();
 		String className = toUpperCamelCase( tableIdentifier.getName() );
 
-		if(pkgName.length()>0) {			
+		if(!pkgName.isEmpty()) {
 			return StringHelper.qualify(pkgName, className);
 		}
 		else {
@@ -187,7 +187,7 @@ public String getOptimisticLockColumnName(TableIdentifier identifier) {
 
 	public boolean useColumnForOptimisticLock(TableIdentifier identifier, String column) {
 		if(settings.getDetectOptimsticLock()) {
-			return AUTO_OPTIMISTICLOCK_COLUMNS.contains(column.toLowerCase())?true:false;
+			return AUTO_OPTIMISTICLOCK_COLUMNS.contains(column.toLowerCase());
 		} else {
 			return false;
 		}
@@ -221,11 +221,7 @@ public boolean isForeignKeyCollectionInverse(String name, Table foreignKeyTable,
 		       // if the reference column is the first one then we are inverse.
 			   Column column = foreignKeyTable.getColumn(0);
 			   Column fkColumn = (Column) referencedColumns.get(0);
-			   if(fkColumn.equals(column)) {
-				   return true;   
-			   } else {
-				   return false;
-			   }
+            return fkColumn.equals(column);
 		}
 		return true;
 	}
@@ -242,26 +238,23 @@ public boolean isOneToOne(ForeignKey foreignKey) {
 		if(settings.getDetectOneToOne()) {
 			// add support for non-PK associations
 			List<Column> fkColumns = foreignKey.getColumns();
-			List<Column> pkForeignTableColumns = null;
-			
+			List<Column> pkForeignTableColumns = Collections.emptyList();
+
 			if (foreignKey.getTable().hasPrimaryKey())
 				pkForeignTableColumns = foreignKey.getTable().getPrimaryKey().getColumns();
 
-			boolean equals =
-				fkColumns != null && pkForeignTableColumns != null
-				&& fkColumns.size() == pkForeignTableColumns.size();
+			boolean equals = fkColumns.size() == pkForeignTableColumns.size();
 
 			Iterator<Column> columns = foreignKey.getColumns().iterator();
 			while (equals && columns.hasNext()) {
-				Column fkColumn = (Column) columns.next();
-				equals = equals && pkForeignTableColumns.contains(fkColumn);
+				Column fkColumn = columns.next();
+				equals = pkForeignTableColumns.contains(fkColumn);
 			}
-
 			return equals;
 		} else {
 			return false;
 		}
-    }
+	}
 
 	public boolean isManyToManyTable(Table table) {
 		if(settings.getDetectManyToMany()) {
@@ -287,14 +280,11 @@ public boolean isManyToManyTable(Table table) {
 			}
 
 			// tests that all columns are implied in the fks
-			Set<Column> columns = new HashSet<Column>();
-			for (Column column : table.getColumns()) {
-				columns.add(column);
-			}
+            Set<Column> columns = new HashSet<>(table.getColumns());
 
 			for (ForeignKey fkey : table.getForeignKeys().values()) {
 				if (columns.isEmpty()) break;
-				columns.removeAll(fkey.getColumns());
+				fkey.getColumns().forEach(columns::remove);
 			}
 
 			return columns.isEmpty();