Fixed: Induction from DB does not represent relations properly. (#290) (OFBIZ-12178)
As encountered in OFBIZ-6510, the ModelInduceFromDb does currently not include entity relations and foreign key constraints. Since they are an important part of the database model, we should fix that.
I could track down the problem to an incomplete invocation of the ModelEntity through the constructor used in the DatabaseUtil.induceModelFromDb() Methods. This constructor does not initialize the Relations.
Problem is, that the ModelEntity initialized through the "DB Names Constructor" does not cover references in its current state at all.
While working on an implementation I realized, that the API is not very congruent in this regards. I would expect that I could initialize ModelRelations the same way ModelFields are initialized in this context:
The create() Method takes a ModelEntity, DatabaseUtil.ColumnCheckInfo (respectively a DatabaseUtil.ReferenceCheckInfo) and a ModelFieldTypeReader (that could be left out for references) and creates ModelField (ModelRelation) objects that are added to the ModelEntity.
But that is not the case at the moment. On one hand not all fields that would be necessary are covered in the DatabaseUtil.ReferenceCheckInfo Objects (e.g. "type" is missing), on the other the object is missing public getters to make the values available in the first place.
Thanks a bunch Benjamin, nice add !
diff --git a/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java b/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java
index 5f2fcb5..531d723 100644
--- a/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java
+++ b/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java
@@ -44,6 +44,7 @@
import org.apache.ofbiz.base.concurrent.ExecutionPool;
import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.base.util.StringUtil;
import org.apache.ofbiz.base.util.UtilTimer;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.entity.GenericEntityException;
@@ -57,6 +58,7 @@
import org.apache.ofbiz.entity.model.ModelIndex;
import org.apache.ofbiz.entity.model.ModelKeyMap;
import org.apache.ofbiz.entity.model.ModelRelation;
+import org.apache.ofbiz.entity.model.ModelUtil;
import org.apache.ofbiz.entity.model.ModelViewEntity;
import org.apache.ofbiz.entity.transaction.TransactionFactoryLoader;
import org.apache.ofbiz.entity.transaction.TransactionUtil;
@@ -861,6 +863,7 @@
if (UtilValidate.isNotEmpty(tableNames)) {
// get ALL column info, put into hashmap by table name
Map<String, Map<String, ColumnCheckInfo>> colInfo = getColumnInfo(tableNames, true, messages, executor);
+ Map<String, Map<String, ReferenceCheckInfo>> refInfo = getReferenceInfo(tableNames, messages);
// go through each table and make a ModelEntity object, add to list
// for each entity make corresponding ModelField objects
// then print out XML for the entities/fields
@@ -868,7 +871,8 @@
// iterate over the table names is alphabetical order
for (String tableName : new TreeSet<>(colInfo.keySet())) {
Map<String, ColumnCheckInfo> colMap = colInfo.get(tableName);
- ModelEntity newEntity = new ModelEntity(tableName, colMap, modelFieldTypeReader, isCaseSensitive);
+ Map<String, ReferenceCheckInfo> refMap = refInfo.get(tableName);
+ ModelEntity newEntity = new ModelEntity(tableName, colMap, refMap, modelFieldTypeReader, isCaseSensitive);
newEntList.add(newEntity);
}
}
@@ -1328,7 +1332,11 @@
* @param messages the messages
* @return the reference info
*/
- public Map<String, Map<String, ReferenceCheckInfo>> getReferenceInfo(Set<String> tableNames, Collection<String> messages) {
+ public Map<String, Map<String, ReferenceCheckInfo>> getReferenceInfo(Set<String> tableNames,
+ Collection<String> messages) {
+ if (UtilValidate.isEmpty(tableNames)) {
+ return new HashMap<>();
+ }
Connection connection = getConnectionLogged(messages);
if (connection == null) {
return null;
@@ -1340,41 +1348,46 @@
} catch (SQLException e) {
String message = "Unable to get database meta data... Error was:" + e.toString();
Debug.logError(message, MODULE);
- if (messages != null) messages.add(message);
+ if (messages != null) {
+ messages.add(message);
+ }
try {
connection.close();
} catch (SQLException e2) {
- String message2 = "Unable to close database connection, continuing anyway... Error was:" + e2.toString();
+ String message2 = "Unable to close database connection, continuing anyway... Error was:" + e2
+ .toString();
Debug.logError(message2, MODULE);
- if (messages != null) messages.add(message2);
+ if (messages != null) {
+ messages.add(message2);
+ }
}
return null;
}
/*
- try {
- if (Debug.infoOn()) Debug.logInfo("Database Product Name is " + dbData.getDatabaseProductName(), MODULE);
- if (Debug.infoOn()) Debug.logInfo("Database Product Version is " + dbData.getDatabaseProductVersion(), MODULE);
- } catch (SQLException e) {
- Debug.logWarning("Unable to get Database name & version information", MODULE);
- }
- try {
- if (Debug.infoOn()) Debug.logInfo("Database Driver Name is " + dbData.getDriverName(), MODULE);
- if (Debug.infoOn()) Debug.logInfo("Database Driver Version is " + dbData.getDriverVersion(), MODULE);
- } catch (SQLException e) {
- Debug.logWarning("Unable to get Driver name & version information", MODULE);
- }
+ * try { if (Debug.infoOn()) Debug.logInfo("Database Product Name is " +
+ * dbData.getDatabaseProductName(), MODULE); if (Debug.infoOn())
+ * Debug.logInfo("Database Product Version is " +
+ * dbData.getDatabaseProductVersion(), MODULE); } catch (SQLException e) {
+ * Debug.logWarning("Unable to get Database name & version information",
+ * MODULE); } try { if (Debug.infoOn()) Debug.logInfo("Database Driver Name is "
+ * + dbData.getDriverName(), MODULE); if (Debug.infoOn())
+ * Debug.logInfo("Database Driver Version is " + dbData.getDriverVersion(),
+ * MODULE); } catch (SQLException e) {
+ * Debug.logWarning("Unable to get Driver name & version information", MODULE);
+ * }
*/
if (Debug.infoOn()) {
Debug.logInfo("Getting Foreign Key (Reference) Info From Database", MODULE);
}
- Map<String, Map<String, ReferenceCheckInfo>> refInfo = new HashMap<>();
+ Map<String, Map<String, ReferenceCheckInfo>> retMap = new HashMap<>();
try {
- // ResultSet rsCols = dbData.getCrossReference(null, null, null, null, null, null);
+ // ResultSet rsCols = dbData.getCrossReference(null, null, null, null, null,
+ // null);
String lookupSchemaName = getSchemaName(dbData);
boolean needsUpperCase = false;
try {
@@ -1382,99 +1395,110 @@
} catch (SQLException e) {
String message = "Error getting identifier case information... Error was:" + e.toString();
Debug.logError(message, MODULE);
- if (messages != null) messages.add(message);
- }
-
- ResultSet rsCols = dbData.getImportedKeys(null, lookupSchemaName, null);
- int totalFkRefs = 0;
-
- // Iterator tableNamesIter = tableNames.iterator();
- // while (tableNamesIter.hasNext()) {
- // String tableName = (String) tableNamesIter.next();
- // ResultSet rsCols = dbData.getImportedKeys(null, null, tableName);
- // Debug.logVerbose("Getting imported keys for table " + tableName, MODULE);
-
- while (rsCols.next()) {
- try {
- ReferenceCheckInfo rcInfo = new ReferenceCheckInfo();
-
- rcInfo.pkTableName = rsCols.getString("PKTABLE_NAME");
- if (needsUpperCase && rcInfo.pkTableName != null) {
- rcInfo.pkTableName = rcInfo.pkTableName.toUpperCase();
- }
- rcInfo.pkColumnName = rsCols.getString("PKCOLUMN_NAME");
- if (needsUpperCase && rcInfo.pkColumnName != null) {
- rcInfo.pkColumnName = rcInfo.pkColumnName.toUpperCase();
- }
-
- rcInfo.fkTableName = rsCols.getString("FKTABLE_NAME");
- if (needsUpperCase && rcInfo.fkTableName != null) {
- rcInfo.fkTableName = rcInfo.fkTableName.toUpperCase();
- }
- // ignore the column info if the FK table name is not in the list we are concerned with
- if (!tableNames.contains(rcInfo.fkTableName)) {
- continue;
- }
- rcInfo.fkColumnName = rsCols.getString("FKCOLUMN_NAME");
- if (needsUpperCase && rcInfo.fkColumnName != null) {
- rcInfo.fkColumnName = rcInfo.fkColumnName.toUpperCase();
- }
- rcInfo.fkName = rsCols.getString("FK_NAME");
- if (needsUpperCase && rcInfo.fkName != null) {
- rcInfo.fkName = rcInfo.fkName.toUpperCase();
- }
-
- if (Debug.verboseOn()) {
- Debug.logVerbose("Got: " + rcInfo.toString(), MODULE);
- }
-
- Map<String, ReferenceCheckInfo> tableRefInfo = refInfo.get(rcInfo.fkTableName);
- if (tableRefInfo == null) {
- tableRefInfo = new HashMap<>();
- refInfo.put(rcInfo.fkTableName, tableRefInfo);
- if (Debug.verboseOn()) {
- Debug.logVerbose("Adding new Map for table: " + rcInfo.fkTableName, MODULE);
- }
- }
- if (!tableRefInfo.containsKey(rcInfo.fkName)) totalFkRefs++;
- tableRefInfo.put(rcInfo.fkName, rcInfo);
- } catch (SQLException e) {
- String message = "Error getting fk reference info for table. Error was:" + e.toString();
- Debug.logError(message, MODULE);
- if (messages != null) messages.add(message);
- continue;
+ if (messages != null) {
+ messages.add(message);
}
}
- // if (Debug.infoOn()) {
- // Debug.logInfo("There are " + totalFkRefs + " in the database", MODULE)
- // };
- try {
- rsCols.close();
- } catch (SQLException e) {
- String message = "Unable to close ResultSet for fk reference list, continuing anyway... Error was:" + e.toString();
- Debug.logError(message, MODULE);
- if (messages != null) messages.add(message);
+ int totalFkRefs = 0;
+
+ for (String tableName : tableNames) {
+ String shortTableName = tableName.split("\\.").length > 0 ? tableName.split("\\.")[tableName.split(
+ "\\.").length - 1] : tableName;
+ ResultSet rsCols = dbData.getImportedKeys(null, lookupSchemaName, shortTableName);
+ // ResultSetMetaData rsMeta = rsCols.getMetaData();
+
+ Map<String, ReferenceCheckInfo> tableRefInfo = retMap.get(tableName);
+ if (tableRefInfo == null) {
+ tableRefInfo = new HashMap<>();
+ totalFkRefs++;
+ }
+ while (rsCols.next()) {
+ try {
+ String fkName = toUpper(rsCols.getString("FK_NAME"), needsUpperCase);
+ int seq = rsCols.getInt("KEY_SEQ");
+ String fkTableName = toUpper(rsCols.getString("FKTABLE_NAME"), needsUpperCase);
+ String fkColumnName = toUpper(rsCols.getString("FKCOLUMN_NAME"), needsUpperCase);
+ String pkTableName = toUpper(rsCols.getString("PKTABLE_NAME"), needsUpperCase);
+ String pkColumnName = toUpper(rsCols.getString("PKCOLUMN_NAME"), needsUpperCase);
+
+ ReferenceCheckInfo rcInfo = tableRefInfo.get(fkName);
+ if (rcInfo == null) {
+ rcInfo = new ReferenceCheckInfo();
+
+ }
+
+ rcInfo.fkName = fkName;
+ rcInfo.fkTableName = fkTableName;
+ rcInfo.pkTableName = pkTableName;
+
+ if (seq > 1) {
+ rcInfo.pkColumnName = rcInfo.pkColumnName.concat(",").concat(pkColumnName);
+ rcInfo.fkColumnName = rcInfo.fkColumnName.concat(",").concat(fkColumnName);
+ } else {
+ rcInfo.pkColumnName = pkColumnName;
+ rcInfo.fkColumnName = fkColumnName;
+ }
+
+ if (Debug.verboseOn()) {
+ Debug.logVerbose("Got: " + rcInfo.toString(), MODULE);
+ }
+ tableRefInfo.put(fkName, rcInfo);
+ retMap.put(tableName, tableRefInfo);
+ } catch (SQLException e) {
+ String message = "Error getting fk reference info for table. Error was:" + e.toString();
+ Debug.logError(message, MODULE);
+ if (messages != null) {
+ messages.add(message);
+ }
+ continue;
+ }
+ }
+ // if (Debug.infoOn()) {
+ // Debug.logInfo("There are " + totalFkRefs + " in the database", MODULE)
+ // };
+ try {
+ rsCols.close();
+ } catch (SQLException e) {
+ String message = "Unable to close ResultSet for fk reference list, continuing anyway... Error was:"
+ + e.toString();
+ Debug.logError(message, MODULE);
+ if (messages != null) {
+ messages.add(message);
+ }
+ }
}
if (Debug.infoOn()) {
Debug.logInfo("There are " + totalFkRefs + " foreign key refs in the database", MODULE);
}
} catch (SQLException e) {
- String message = "Error getting fk reference meta data Error was:" + e.toString() + ". Not checking fk refs.";
+ String message = "Error getting fk reference meta data Error was:" + e.toString()
+ + ". Not checking fk refs.";
Debug.logError(message, MODULE);
- if (messages != null) messages.add(message);
- refInfo = null;
+ if (messages != null) {
+ messages.add(message);
+ }
+ retMap = null;
} finally {
try {
connection.close();
} catch (SQLException e) {
String message = "Unable to close database connection, continuing anyway... Error was:" + e.toString();
Debug.logError(message, MODULE);
- if (messages != null) messages.add(message);
+ if (messages != null) {
+ messages.add(message);
+ }
}
}
- return refInfo;
+ return retMap;
+ }
+
+ private String toUpper(String string, boolean needsUpperCase) {
+ if (needsUpperCase && string != null) {
+ string = string.toUpperCase();
+ }
+ return string;
}
/**
@@ -3230,9 +3254,79 @@
*/
private String fkColumnName;
+ /**
+ * Gets pk table name
+ * @return the pk table name
+ */
+ public String getPkTableName() {
+ return pkTableName;
+ }
+
+ /**
+ * Gets pk column name
+ * @return the pk column name
+ */
+ public String getPkColumnName() {
+ return pkColumnName;
+ }
+
+ /**
+ * Gets fk name
+ * @return the fk name
+ */
+ public String getFkName() {
+ return fkName;
+ }
+
+ /**
+ * Gets fk table name
+ * @return the fk table name
+ */
+ public String getFkTableName() {
+ return fkTableName;
+ }
+
+ /**
+ * Gets fk column name
+ * @return the fk column name
+ */
+ public String getFkColumnName() {
+ return fkColumnName;
+ }
+
@Override
public String toString() {
- return "FK Reference from table " + fkTableName + " called " + fkName + " to PK in table " + pkTableName;
+ StringBuilder ret = new StringBuilder();
+ ret.append("FK Reference from table ");
+ ret.append(fkTableName);
+ ret.append(" called ");
+ ret.append(fkName);
+ ret.append(" to PK in table ");
+ ret.append(pkTableName);
+ return ret.toString();
+ }
+
+ /**
+ * Converts the column information into ModelKeyMaps
+ * @return a list of ModelKeyMaps
+ */
+ public List<ModelKeyMap> toModelKeyMapList() {
+ List<ModelKeyMap> keyMaps = new ArrayList<>();
+
+ List<String> fieldNames = StringUtil.split(fkColumnName, ",");
+ List<String> relFieldNames = StringUtil.split(pkColumnName, ",");
+ if (fieldNames.size() != relFieldNames.size()) {
+ Debug.logError("Count of related fields for relation does not match!", MODULE);
+ return null;
+ }
+ for (int i = 0; i < fieldNames.size(); i++) {
+ String fieldName = ModelUtil.dbNameToVarName(fieldNames.get(i));
+ String relFieldName = ModelUtil.dbNameToVarName(relFieldNames.get(i));
+ ModelKeyMap keyMap = new ModelKeyMap(fieldName, relFieldName);
+ keyMaps.add(keyMap);
+ }
+
+ return keyMaps;
}
}
diff --git a/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelEntity.java b/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelEntity.java
index 83e44e7..c7b8fe6 100644
--- a/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelEntity.java
+++ b/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelEntity.java
@@ -247,6 +247,33 @@
}
}
+ public ModelEntity(String tableName, Map<String, DatabaseUtil.ColumnCheckInfo> colMap,
+ Map<String, DatabaseUtil.ReferenceCheckInfo> refMap, ModelFieldTypeReader modelFieldTypeReader,
+ boolean isCaseSensitive) {
+ // if there is a dot in the name, remove it and everything before it, should be
+ // the schema name
+ this.modelReader = null;
+ this.modelInfo = ModelInfo.DEFAULT;
+ this.tableName = tableName;
+ int dotIndex = this.tableName.indexOf('.');
+ if (dotIndex >= 0) {
+ this.tableName = this.tableName.substring(dotIndex + 1);
+ }
+ this.entityName = ModelUtil.dbNameToClassName(this.tableName);
+ for (Map.Entry<String, DatabaseUtil.ColumnCheckInfo> columnEntry : colMap.entrySet()) {
+ DatabaseUtil.ColumnCheckInfo ccInfo = columnEntry.getValue();
+ ModelField newField = ModelField.create(this, ccInfo, modelFieldTypeReader);
+ addField(newField);
+ }
+ if (UtilValidate.isNotEmpty(refMap)) {
+ for (Map.Entry<String, DatabaseUtil.ReferenceCheckInfo> referenceEntry : refMap.entrySet()) {
+ DatabaseUtil.ReferenceCheckInfo rcInfo = referenceEntry.getValue();
+ ModelRelation newRel = ModelRelation.create(this, rcInfo, false);
+ addRelation(newRel);
+ }
+ }
+ }
+
/**
* Populate basic info.
* @param entityElement the entity element
diff --git a/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelRelation.java b/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelRelation.java
index ba429b5..9c0d5aa 100644
--- a/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelRelation.java
+++ b/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelRelation.java
@@ -28,6 +28,7 @@
import org.apache.ofbiz.base.lang.ThreadSafe;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.base.util.UtilXml;
+import org.apache.ofbiz.entity.jdbc.DatabaseUtil;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
@@ -39,6 +40,34 @@
@SuppressWarnings("serial")
public final class ModelRelation extends ModelChild {
+ /*
+ * Developers - this is an immutable class. Once constructed, the object should not change state.
+ * Therefore, 'setter' methods are not allowed. If client code needs to modify the object's
+ * state, then it can create a new copy with the changed values.
+ */
+
+ /** the title, gives a name/description to the relation */
+ private final String title;
+
+ /** the type: either "one" or "many" or "one-nofk" */
+ private final String type;
+
+ /** the name of the related entity */
+ private final String relEntityName;
+
+ /** the name to use for a database foreign key, if applies */
+ private final String fkName;
+
+ /** keyMaps defining how to lookup the relatedTable using columns from this table */
+ private final List<ModelKeyMap> keyMaps;
+
+ private final boolean isAutoRelation;
+
+ /** A String to uniquely identify this relation. */
+ private final String fullName;
+
+ private final String combinedName;
+
/**
* Returns a new <code>ModelRelation</code> instance, initialized with the specified values.
* @param modelEntity The <code>ModelEntity</code> this relation is a member of.
@@ -99,33 +128,16 @@
return new ModelRelation(modelEntity, description, type, title, relEntityName, fkName, keyMaps, isAutoRelation);
}
- /*
- * Developers - this is an immutable class. Once constructed, the object should not change state.
- * Therefore, 'setter' methods are not allowed. If client code needs to modify the object's
- * state, then it can create a new copy with the changed values.
- */
+ public static ModelRelation create(ModelEntity modelEntity, DatabaseUtil.ReferenceCheckInfo refInfo, boolean isAutoRelation) {
+ String fkName = refInfo.getFkName();
+ String title = null;
+ String type = null;
+ String description = null;
+ String relEntityName = ModelUtil.dbNameToClassName(refInfo.getPkTableName());
+ List<ModelKeyMap> keyMaps = refInfo.toModelKeyMapList();
- /** the title, gives a name/description to the relation */
- private final String title;
-
- /** the type: either "one" or "many" or "one-nofk" */
- private final String type;
-
- /** the name of the related entity */
- private final String relEntityName;
-
- /** the name to use for a database foreign key, if applies */
- private final String fkName;
-
- /** keyMaps defining how to lookup the relatedTable using columns from this table */
- private final List<ModelKeyMap> keyMaps;
-
- private final boolean isAutoRelation;
-
- /** A String to uniquely identify this relation. */
- private final String fullName;
-
- private final String combinedName;
+ return new ModelRelation(modelEntity, description, type, title, relEntityName, fkName, keyMaps, isAutoRelation);
+ }
private ModelRelation(ModelEntity modelEntity, String description, String type, String title, String relEntityName, String fkName,
List<ModelKeyMap> keyMaps, boolean isAutoRelation) {
@@ -149,7 +161,7 @@
}
sb.append("]");
this.fullName = sb.toString();
- this.combinedName = title.concat(relEntityName);
+ this.combinedName = title != null ? title.concat(relEntityName) : relEntityName;
}
/** Returns the combined name (title + related entity name). */
diff --git a/framework/webtools/config/WebtoolsUiLabels.xml b/framework/webtools/config/WebtoolsUiLabels.xml
index 863e2c5..22f7d85 100644
--- a/framework/webtools/config/WebtoolsUiLabels.xml
+++ b/framework/webtools/config/WebtoolsUiLabels.xml
@@ -6041,4 +6041,12 @@
<value xml:lang="de">Ermittlung der Datenstruktur fehlgeschlagen. Bitte gültige Datenquelle angeben.</value>
<value xml:lang="en">Could not determine Structure for this datasource. Please choose a valid datasource.</value>
</property>
+ <property key="typeAndTitleDisclaimer">
+ <value xml:lang="de">Bitte beachten Sie, dass die Attribute Type und Title der Referenzen nicht aus der Datenbank heraus rekonstruiert werden können.</value>
+ <value xml:lang="en">Please note that the attributes Type and Title for references cannot be derived from the database.</value>
+ </property>
+ <property key="pleaseAddManually">
+ <value xml:lang="de">Diese müssen händisch nachgepflegt werden.</value>
+ <value xml:lang="en">These must be added manually.</value>
+ </property>
</resource>
diff --git a/framework/webtools/template/entity/ModelInduceFromDb.ftl b/framework/webtools/template/entity/ModelInduceFromDb.ftl
index bb40ce6..c0c5ca1 100644
--- a/framework/webtools/template/entity/ModelInduceFromDb.ftl
+++ b/framework/webtools/template/entity/ModelInduceFromDb.ftl
@@ -54,6 +54,9 @@
</table>
</form>
</hr>
+<div>${uiLabelMap.typeAndTitleDisclaimer}</div>
+<div>${uiLabelMap.pleaseAddManually}</div>
+</hr>
<div>
<textarea cols="60" rows="50" name="${uiLabelMap.ModelInduceInducedText}">${inducedText!}</textarea>
</div>
