CAY-2113 cdbimport: Reverse-engineering reinstates previously ignored columns
* fixing... Project: http://git-wip-us.apache.org/repos/asf/cayenne/repo Commit: http://git-wip-us.apache.org/repos/asf/cayenne/commit/f7f33b55 Tree: http://git-wip-us.apache.org/repos/asf/cayenne/tree/f7f33b55 Diff: http://git-wip-us.apache.org/repos/asf/cayenne/diff/f7f33b55 Branch: refs/heads/master Commit: f7f33b5570dc704d88322e2ac08bda677050a174 Parents: d58f722 Author: Andrus Adamchik <and...@objectstyle.com> Authored: Wed Sep 28 16:45:56 2016 +0300 Committer: Andrus Adamchik <and...@objectstyle.com> Committed: Wed Sep 28 18:02:35 2016 +0300 ---------------------------------------------------------------------- .../cayenne/merge/AbstractToModelToken.java | 7 - .../apache/cayenne/merge/AddColumnToModel.java | 10 +- .../cayenne/merge/AddRelationshipToModel.java | 14 +- .../cayenne/merge/CreateTableToModel.java | 10 +- .../apache/cayenne/util/EntityMergeSupport.java | 277 +++++++++++-------- docs/doc/src/main/resources/RELEASE-NOTES.txt | 1 + 6 files changed, 186 insertions(+), 133 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cayenne/blob/f7f33b55/cayenne-server/src/main/java/org/apache/cayenne/merge/AbstractToModelToken.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/merge/AbstractToModelToken.java b/cayenne-server/src/main/java/org/apache/cayenne/merge/AbstractToModelToken.java index e2ca05f..49b680b 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/merge/AbstractToModelToken.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/merge/AbstractToModelToken.java @@ -23,7 +23,6 @@ import org.apache.cayenne.map.DbEntity; import org.apache.cayenne.map.DbRelationship; import org.apache.cayenne.map.ObjEntity; import org.apache.cayenne.map.ObjRelationship; -import org.apache.cayenne.util.EntityMergeSupport; /** * Common abstract superclass for all {@link MergerToken}s going from the database to the @@ -47,12 +46,6 @@ public abstract class AbstractToModelToken implements MergerToken { return MergeDirection.TO_MODEL; } - protected void synchronizeWithObjEntity(DbEntity entity) { - for (ObjEntity objEntity : entity.mappedObjEntities()) { - new EntityMergeSupport(objEntity.getDataMap()).synchronizeWithDbEntity(objEntity); - } - } - protected static void remove(ModelMergeDelegate mergerContext, DbRelationship rel, boolean reverse) { if (rel == null) { return; http://git-wip-us.apache.org/repos/asf/cayenne/blob/f7f33b55/cayenne-server/src/main/java/org/apache/cayenne/merge/AddColumnToModel.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/merge/AddColumnToModel.java b/cayenne-server/src/main/java/org/apache/cayenne/merge/AddColumnToModel.java index 283241c..9e96928 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/merge/AddColumnToModel.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/merge/AddColumnToModel.java @@ -40,7 +40,15 @@ public class AddColumnToModel extends AbstractToModelToken.EntityAndColumn { public void execute(MergerContext mergerContext) { getEntity().addAttribute(getColumn()); - synchronizeWithObjEntity(getEntity()); + + // TODO: use EntityMergeSupport from DbImportConfiguration... otherwise we are ignoring a bunch of + // important settings + + EntityMergeSupport entityMergeSupport = new EntityMergeSupport(mergerContext.getDataMap()); + for(ObjEntity e : getEntity().mappedObjEntities()) { + entityMergeSupport.synchronizeOnDbAttributeAdded(e, getColumn()); + } + mergerContext.getModelMergeDelegate().dbAttributeAdded(getColumn()); } http://git-wip-us.apache.org/repos/asf/cayenne/blob/f7f33b55/cayenne-server/src/main/java/org/apache/cayenne/merge/AddRelationshipToModel.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/merge/AddRelationshipToModel.java b/cayenne-server/src/main/java/org/apache/cayenne/merge/AddRelationshipToModel.java index 5e05b52..42d7329 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/merge/AddRelationshipToModel.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/merge/AddRelationshipToModel.java @@ -21,8 +21,8 @@ package org.apache.cayenne.merge; import org.apache.cayenne.map.DbEntity; import org.apache.cayenne.map.DbJoin; import org.apache.cayenne.map.DbRelationship; - -import static org.apache.cayenne.util.Util.join; +import org.apache.cayenne.map.ObjEntity; +import org.apache.cayenne.util.EntityMergeSupport; public class AddRelationshipToModel extends AbstractToModelToken.Entity { @@ -42,7 +42,15 @@ public class AddRelationshipToModel extends AbstractToModelToken.Entity { public void execute(MergerContext mergerContext) { getEntity().addRelationship(rel); // TODO: add reverse relationship as well if it does not exist - synchronizeWithObjEntity(getEntity()); + + // TODO: use EntityMergeSupport from DbImportConfiguration... otherwise we are ignoring a bunch of + // important settings + + EntityMergeSupport entityMergeSupport = new EntityMergeSupport(mergerContext.getDataMap()); + for(ObjEntity e : getEntity().mappedObjEntities()) { + entityMergeSupport.synchronizeOnDbRelationshipAdded(e, rel); + } + mergerContext.getModelMergeDelegate().dbRelationshipAdded(rel); } http://git-wip-us.apache.org/repos/asf/cayenne/blob/f7f33b55/cayenne-server/src/main/java/org/apache/cayenne/merge/CreateTableToModel.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/merge/CreateTableToModel.java b/cayenne-server/src/main/java/org/apache/cayenne/merge/CreateTableToModel.java index 61a266a..cdd6009 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/merge/CreateTableToModel.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/merge/CreateTableToModel.java @@ -22,7 +22,7 @@ import org.apache.cayenne.map.DataMap; import org.apache.cayenne.map.DbEntity; import org.apache.cayenne.map.ObjEntity; import org.apache.cayenne.map.naming.NameConverter; -import org.apache.cayenne.util.Util; +import org.apache.cayenne.util.EntityMergeSupport; /** * A {@link MergerToken} to add a {@link DbEntity} to a {@link DataMap} @@ -85,8 +85,12 @@ public class CreateTableToModel extends AbstractToModelToken.Entity { map.addObjEntity(objEntity); - synchronizeWithObjEntity(getEntity()); - + // presumably there are no other ObjEntities pointing to this DbEntity, so syncing just this one... + + // TODO: use EntityMergeSupport from DbImportConfiguration... otherwise we are ignoring a bunch of + // important settings + new EntityMergeSupport(map).synchronizeWithDbEntity(objEntity); + mergerContext.getModelMergeDelegate().dbEntityAdded(getEntity()); mergerContext.getModelMergeDelegate().objEntityAdded(objEntity); } http://git-wip-us.apache.org/repos/asf/cayenne/blob/f7f33b55/cayenne-server/src/main/java/org/apache/cayenne/util/EntityMergeSupport.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/util/EntityMergeSupport.java b/cayenne-server/src/main/java/org/apache/cayenne/util/EntityMergeSupport.java index 5bcccce..7c12600 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/util/EntityMergeSupport.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/util/EntityMergeSupport.java @@ -19,13 +19,6 @@ package org.apache.cayenne.util; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; - import org.apache.cayenne.dba.TypesMapping; import org.apache.cayenne.map.DataMap; import org.apache.cayenne.map.DbAttribute; @@ -36,13 +29,20 @@ import org.apache.cayenne.map.Entity; import org.apache.cayenne.map.ObjAttribute; import org.apache.cayenne.map.ObjEntity; import org.apache.cayenne.map.ObjRelationship; -import org.apache.cayenne.map.naming.LegacyNameGenerator; import org.apache.cayenne.map.naming.DefaultUniqueNameGenerator; +import org.apache.cayenne.map.naming.LegacyNameGenerator; import org.apache.cayenne.map.naming.NameCheckers; import org.apache.cayenne.map.naming.ObjectNameGenerator; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + /** * Implements methods for entity merging. */ @@ -64,20 +64,17 @@ public class EntityMergeSupport { } private final DataMap map; - - protected boolean removeMeaningfulFKs; - protected boolean removeMeaningfulPKs; - protected boolean usePrimitives; - /** * Strategy for choosing names for entities, attributes and relationships */ private final ObjectNameGenerator nameGenerator; - /** * Listeners of merge process. */ private final List<EntityMergeListener> listeners = new ArrayList<EntityMergeListener>(); + protected boolean removeMeaningfulFKs; + protected boolean removeMeaningfulPKs; + protected boolean usePrimitives; public EntityMergeSupport(DataMap map) { this(map, new LegacyNameGenerator(), true); @@ -102,7 +99,7 @@ public class EntityMergeSupport { /** * Updates each one of the collection of ObjEntities, adding attributes and * relationships based on the current state of its DbEntity. - * + * * @return true if any ObjEntity has changed as a result of synchronization. * @since 1.2 changed signature to use Collection instead of List. */ @@ -134,7 +131,7 @@ public class EntityMergeSupport { /** * Updates ObjEntity attributes and relationships based on the current state * of its DbEntity. - * + * * @return true if the ObjEntity has changed as a result of synchronization. */ public boolean synchronizeWithDbEntity(ObjEntity entity) { @@ -165,6 +162,32 @@ public class EntityMergeSupport { return changed; } + /** + * @since 4.0 + */ + public boolean synchronizeOnDbAttributeAdded(ObjEntity entity, DbAttribute dbAttribute) { + + Collection<DbRelationship> incomingRels = getIncomingRelationships(dbAttribute.getEntity()); + if (isMissingFromObjEntity(entity, dbAttribute, incomingRels)) { + addMissingAttribute(entity, dbAttribute); + return true; + } + + return false; + } + + /** + * @since 4.0 + */ + public boolean synchronizeOnDbRelationshipAdded(ObjEntity entity, DbRelationship dbRelationship) { + + if (isMissingFromObjEntity(entity, dbRelationship)) { + addMissingRelationship(entity, dbRelationship); + } + + return true; + } + private boolean addMissingRelationships(ObjEntity entity) { List<DbRelationship> relationshipsToAdd = getRelationshipsToAdd(entity); if (relationshipsToAdd.isEmpty()) { @@ -172,26 +195,9 @@ public class EntityMergeSupport { } for (DbRelationship dr : relationshipsToAdd) { - DbEntity targetEntity = dr.getTargetEntity(); - - Collection<ObjEntity> mappedObjEntities = map.getMappedEntities(targetEntity); - if (!mappedObjEntities.isEmpty()) { - for (Entity mappedTarget : mappedObjEntities) { - createObjRelationship(entity, dr, mappedTarget.getName()); - } - } else { - if (targetEntity == null) { - targetEntity = new DbEntity(dr.getTargetEntityName()); - } - if (dr.getTargetEntityName() != null) { - boolean needGeneratedEntity = createObjRelationship(entity, dr, nameGenerator.createObjEntityName(targetEntity)); - if (needGeneratedEntity) { - LOG.warn("Can't find ObjEntity for " + dr.getTargetEntityName()); - LOG.warn("Db Relationship (" + dr + ") will have GUESSED Obj Relationship reflection. "); - } - } - } + addMissingRelationship(entity, dr); } + return true; } @@ -237,26 +243,55 @@ public class EntityMergeSupport { private boolean addMissingAttributes(ObjEntity entity) { boolean changed = false; + for (DbAttribute da : getAttributesToAdd(entity)) { + addMissingAttribute(entity, da); + changed = true; + } + return changed; + } - String attrName = DefaultUniqueNameGenerator.generate(NameCheckers.objAttribute, entity, - nameGenerator.createObjAttributeName(da)); + private void addMissingRelationship(ObjEntity entity, DbRelationship dbRelationship) { + DbEntity targetEntity = dbRelationship.getTargetEntity(); + + Collection<ObjEntity> mappedObjEntities = map.getMappedEntities(targetEntity); + if (!mappedObjEntities.isEmpty()) { + for (Entity mappedTarget : mappedObjEntities) { + createObjRelationship(entity, dbRelationship, mappedTarget.getName()); + } + } else { - String type = TypesMapping.getJavaBySqlType(da.getType()); - if (usePrimitives) { - String primitive = CLASS_TO_PRIMITIVE.get(type); - if (primitive != null) { - type = primitive; + if (targetEntity == null) { + targetEntity = new DbEntity(dbRelationship.getTargetEntityName()); + } + + if (dbRelationship.getTargetEntityName() != null) { + boolean needGeneratedEntity = createObjRelationship(entity, dbRelationship, + nameGenerator.createObjEntityName(targetEntity)); + if (needGeneratedEntity) { + LOG.warn("Can't find ObjEntity for " + dbRelationship.getTargetEntityName()); + LOG.warn("Db Relationship (" + dbRelationship + ") will have GUESSED Obj Relationship reflection. "); } } + } + } - ObjAttribute oa = new ObjAttribute(attrName, type, entity); - oa.setDbAttributePath(da.getName()); - entity.addAttribute(oa); - fireAttributeAdded(oa); - changed = true; + private void addMissingAttribute(ObjEntity entity, DbAttribute da) { + String attrName = DefaultUniqueNameGenerator.generate(NameCheckers.objAttribute, entity, + nameGenerator.createObjAttributeName(da)); + + String type = TypesMapping.getJavaBySqlType(da.getType()); + if (usePrimitives) { + String primitive = CLASS_TO_PRIMITIVE.get(type); + if (primitive != null) { + type = primitive; + } } - return changed; + + ObjAttribute oa = new ObjAttribute(attrName, type, entity); + oa.setDbAttributePath(da.getName()); + entity.addAttribute(oa); + fireAttributeAdded(oa); } private boolean getRidOfAttributesThatAreNowSrcAttributesForRelationships(ObjEntity entity) { @@ -275,7 +310,7 @@ public class EntityMergeSupport { /** * Returns a list of DbAttributes that are mapped to foreign keys. - * + * * @since 1.2 */ public Collection<DbAttribute> getMeaningfulFKs(ObjEntity objEntity) { @@ -301,102 +336,106 @@ public class EntityMergeSupport { DbEntity dbEntity = objEntity.getDbEntity(); List<DbAttribute> missing = new ArrayList<DbAttribute>(); - - Collection<DbRelationship> rels = dbEntity.getRelationships(); Collection<DbRelationship> incomingRels = getIncomingRelationships(dbEntity); for (DbAttribute dba : dbEntity.getAttributes()) { - if (dba.getName() == null || objEntity.getAttributeForDbAttribute(dba) != null) { - continue; + if (isMissingFromObjEntity(objEntity, dba, incomingRels)) { + missing.add(dba); } + } - boolean removeMeaningfulPKs = removePK(dbEntity); - if (removeMeaningfulPKs && dba.isPrimaryKey()) { - continue; - } + return missing; + } - // check FK's - boolean isFK = false; - Iterator<DbRelationship> rit = rels.iterator(); - while (!isFK && rit.hasNext()) { - DbRelationship rel = rit.next(); - for (DbJoin join : rel.getJoins()) { - if (join.getSource() == dba) { - isFK = true; - break; - } - } - } + protected boolean isMissingFromObjEntity(ObjEntity entity, DbAttribute dbAttribute, Collection<DbRelationship> incomingRels) { - if (!removeMeaningfulPKs) { - if (!dba.isPrimaryKey() && isFK) { - continue; - } - } else { - if (isFK) { - continue; + if (dbAttribute.getName() == null || entity.getAttributeForDbAttribute(dbAttribute) != null) { + return false; + } + + boolean removeMeaningfulPKs = removePK(dbAttribute.getEntity()); + if (removeMeaningfulPKs && dbAttribute.isPrimaryKey()) { + return false; + } + + // check FK's + boolean isFK = false; + Iterator<DbRelationship> rit = dbAttribute.getEntity().getRelationships().iterator(); + while (!isFK && rit.hasNext()) { + DbRelationship rel = rit.next(); + for (DbJoin join : rel.getJoins()) { + if (join.getSource() == dbAttribute) { + isFK = true; + break; } } + } - // check incoming relationships - rit = incomingRels.iterator(); - while (!isFK && rit.hasNext()) { - DbRelationship rel = rit.next(); - for (DbJoin join : rel.getJoins()) { - if (join.getTarget() == dba) { - isFK = true; - break; - } - } + if (!removeMeaningfulPKs) { + if (!dbAttribute.isPrimaryKey() && isFK) { + return false; } + } else { + if (isFK) { + return false; + } + } - if (!removeMeaningfulPKs) { - if (!dba.isPrimaryKey() && isFK) { - continue; - } - } else { - if (isFK) { - continue; + // check incoming relationships + rit = incomingRels.iterator(); + while (!isFK && rit.hasNext()) { + DbRelationship rel = rit.next(); + for (DbJoin join : rel.getJoins()) { + if (join.getTarget() == dbAttribute) { + isFK = true; + break; } } + } - missing.add(dba); + if (!removeMeaningfulPKs) { + if (!dbAttribute.isPrimaryKey() && isFK) { + return false; + } + } else { + if (isFK) { + return false; + } } - return missing; + return true; } - private Collection<DbRelationship> getIncomingRelationships(DbEntity entity) { - Collection<DbRelationship> incoming = new ArrayList<DbRelationship>(); + protected boolean isMissingFromObjEntity(ObjEntity entity, DbRelationship dbRelationship) { + return dbRelationship.getName() != null && entity.getRelationshipForDbRelationship(dbRelationship) == null; + } - for (DbEntity nextEntity : entity.getDataMap().getDbEntities()) { - for (DbRelationship relationship : nextEntity.getRelationships()) { + private Collection<DbRelationship> getIncomingRelationships(DbEntity entity) { + Collection<DbRelationship> incoming = new ArrayList<DbRelationship>(); - // TODO: PERFORMANCE 'getTargetEntity' is generally slow, called - // in this iterator it is showing (e.g. in YourKit profiles).. - // perhaps use cheaper 'getTargetEntityName()' or even better - - // pre-cache all relationships by target entity to avoid O(n) - // search ? - // (need to profile to prove the difference) - if (entity == relationship.getTargetEntity()) { - incoming.add(relationship); - } - } - } + for (DbEntity nextEntity : entity.getDataMap().getDbEntities()) { + for (DbRelationship relationship : nextEntity.getRelationships()) { - return incoming; - } + // TODO: PERFORMANCE 'getTargetEntity' is generally slow, called + // in this iterator it is showing (e.g. in YourKit profiles).. + // perhaps use cheaper 'getTargetEntityName()' or even better - + // pre-cache all relationships by target entity to avoid O(n) + // search ? + // (need to profile to prove the difference) + if (entity == relationship.getTargetEntity()) { + incoming.add(relationship); + } + } + } + + return incoming; + } protected List<DbRelationship> getRelationshipsToAdd(ObjEntity objEntity) { List<DbRelationship> missing = new ArrayList<DbRelationship>(); for (DbRelationship dbRel : objEntity.getDbEntity().getRelationships()) { - // check if adding it makes sense at all - if (dbRel.getName() == null) { - continue; - } - - if (objEntity.getRelationshipForDbRelationship(dbRel) == null) { + if (isMissingFromObjEntity(objEntity, dbRel)) { missing.add(dbRel); } } @@ -472,8 +511,8 @@ public class EntityMergeSupport { } /** - * @since 4.0 * @param usePrimitives + * @since 4.0 */ public void setUsePrimitives(boolean usePrimitives) { this.usePrimitives = usePrimitives; http://git-wip-us.apache.org/repos/asf/cayenne/blob/f7f33b55/docs/doc/src/main/resources/RELEASE-NOTES.txt ---------------------------------------------------------------------- diff --git a/docs/doc/src/main/resources/RELEASE-NOTES.txt b/docs/doc/src/main/resources/RELEASE-NOTES.txt index d6af992..aa04b5f 100644 --- a/docs/doc/src/main/resources/RELEASE-NOTES.txt +++ b/docs/doc/src/main/resources/RELEASE-NOTES.txt @@ -31,6 +31,7 @@ CAY-2106 cayenne-crypto: allow DI contribution of type converters inside ValueTr CAY-2107 cayenne-crypto: Lazy initialization of crypto subsystem CAY-2111 Unbind transaction object from the current thread for iterated queries CAY-2112 Expose callback for "performInTransaction" +CAY-2113 cdbimport: Reverse-engineering reinstates previously ignored columns Bug Fixes:
