cayenne-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From skolbac...@apache.org
Subject [1/2] cayenne git commit: CAY-2153 After reverse engineering is done all data map is checked for invalid references from Obj to Db attributes Additionally some defensive code added to bypass invalid db path state in ObjAttribute: - correct check for this
Date Thu, 24 Nov 2016 14:54:50 GMT
Repository: cayenne
Updated Branches:
  refs/heads/master 00b8d3204 -> 31f828258


CAY-2153
After reverse engineering is done all data map is checked for invalid references from Obj
to Db attributes
Additionally some defensive code added to bypass invalid db path state in ObjAttribute:
- correct check for this state in ObjAttribute validator
- returning null in ObjAttributeWrapper in modeler
- unconditional saving of db attribute path to XML (this one is questionable though)


Project: http://git-wip-us.apache.org/repos/asf/cayenne/repo
Commit: http://git-wip-us.apache.org/repos/asf/cayenne/commit/bd522367
Tree: http://git-wip-us.apache.org/repos/asf/cayenne/tree/bd522367
Diff: http://git-wip-us.apache.org/repos/asf/cayenne/diff/bd522367

Branch: refs/heads/master
Commit: bd522367354b0943178a9fc3a77be9ad0381f3e5
Parents: f527275
Author: Nikita Timofeev <stariy95@gmail.com>
Authored: Thu Nov 24 14:24:57 2016 +0300
Committer: Nikita Timofeev <stariy95@gmail.com>
Committed: Thu Nov 24 14:24:57 2016 +0300

----------------------------------------------------------------------
 .../validation/ObjAttributeValidator.java       | 200 +++++++++----------
 .../org/apache/cayenne/map/ObjAttribute.java    |   4 +-
 docs/doc/src/main/resources/RELEASE-NOTES.txt   |   1 +
 .../modeler/dialog/db/DbLoaderHelper.java       |   2 +
 .../editor/wrapper/ObjAttributeWrapper.java     |   7 +-
 5 files changed, 109 insertions(+), 105 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cayenne/blob/bd522367/cayenne-project/src/main/java/org/apache/cayenne/project/validation/ObjAttributeValidator.java
----------------------------------------------------------------------
diff --git a/cayenne-project/src/main/java/org/apache/cayenne/project/validation/ObjAttributeValidator.java
b/cayenne-project/src/main/java/org/apache/cayenne/project/validation/ObjAttributeValidator.java
index 7671bbe..7f58e32 100644
--- a/cayenne-project/src/main/java/org/apache/cayenne/project/validation/ObjAttributeValidator.java
+++ b/cayenne-project/src/main/java/org/apache/cayenne/project/validation/ObjAttributeValidator.java
@@ -20,6 +20,8 @@ package org.apache.cayenne.project.validation;
 
 import java.util.Map;
 
+import org.apache.cayenne.exp.ExpressionException;
+import org.apache.cayenne.map.DbAttribute;
 import org.apache.cayenne.map.Embeddable;
 import org.apache.cayenne.map.EmbeddableAttribute;
 import org.apache.cayenne.map.EmbeddedAttribute;
@@ -32,124 +34,120 @@ class ObjAttributeValidator extends ConfigurationNodeValidator {
 
     void validate(ObjAttribute attribute, ValidationResult validationResult) {
 
-        // Must have name
-        if (Util.isEmptyString(attribute.getName())) {
-            addFailure(validationResult, attribute, "Unnamed ObjAttribute");
-        }
-        else {
-            NameValidationHelper helper = NameValidationHelper.getInstance();
-            String invalidChars = helper.invalidCharsInObjPathComponent(attribute
-                    .getName());
-
-            if (invalidChars != null) {
-                addFailure(
-                        validationResult,
-                        attribute,
-                        "ObjAttribute name '%s' contains invalid characters: %s",
-                        attribute.getName(),
-                        invalidChars);
-            }
-            else if (helper.invalidDataObjectProperty(attribute.getName())) {
-                addFailure(
-                        validationResult,
-                        attribute,
-                        "ObjAttribute name '%s' is invalid",
-                        attribute.getName());
-            }
-        }
+        validateName(attribute, validationResult);
 
         // all attributes must have type
         if (Util.isEmptyString(attribute.getType())) {
-            addFailure(
-                    validationResult,
-                    attribute,
+            addFailure(validationResult, attribute,
                     "ObjAttribute '%s' has no Java type",
                     attribute.getName());
         }
 
-        if (attribute.getEntity() instanceof ObjEntity
-                && ((ObjEntity) attribute.getEntity()).isAbstract()) {
-            // nothing, abstract entity does not have to define a dbAttribute
+        if (attribute instanceof EmbeddedAttribute) {
+            validateEmbeddable((EmbeddedAttribute)attribute, validationResult);
+        } else {
+            validateDbAttribute(attribute, validationResult);
         }
-        else if (attribute instanceof EmbeddedAttribute) {
-            Map<String, String> attrOverrides = ((EmbeddedAttribute) attribute)
-                    .getAttributeOverrides();
-
-            Embeddable embeddable = ((EmbeddedAttribute) attribute).getEmbeddable();
-            if (embeddable == null && ((EmbeddedAttribute) attribute).getType() !=
null) {
 
-                addFailure(
-                        validationResult,
-                        attribute,
-                        "EmbeddedAttribute '%s' has incorrect Embeddable",
-                        attribute.getName());
-            }
-            else if (embeddable == null
-                    && ((EmbeddedAttribute) attribute).getType() == null) {
-                addFailure(
-                        validationResult,
-                        attribute,
-                        "EmbeddedAttribute '%s' has no Embeddable",
-                        attribute.getName());
-            }
+        checkForDuplicates(attribute, validationResult);
+    }
 
-            if (embeddable != null) {
+    private void validateName(ObjAttribute attribute, ValidationResult validationResult)
{
+        // Must have name
+        if (Util.isEmptyString(attribute.getName())) {
+            addFailure(validationResult, attribute, "Unnamed ObjAttribute");
+            return;
+        }
 
-                for (EmbeddableAttribute embeddableAttribute : embeddable.getAttributes())
{
-                    String dbAttributeName;
-                    if (attrOverrides.size() > 0
-                            && attrOverrides.containsKey(embeddableAttribute.getName()))
{
-                        dbAttributeName = attrOverrides
-                                .get(embeddableAttribute.getName());
-                    }
-                    else {
-                        dbAttributeName = embeddableAttribute.getDbAttributeName();
-                    }
+        NameValidationHelper helper = NameValidationHelper.getInstance();
+        String invalidChars = helper.invalidCharsInObjPathComponent(attribute.getName());
 
-                    if (dbAttributeName == "" || dbAttributeName == null) {
+        if (invalidChars != null) {
+            addFailure(validationResult, attribute,
+                    "ObjAttribute name '%s' contains invalid characters: %s",
+                    attribute.getName(),
+                    invalidChars);
+        } else if (helper.invalidDataObjectProperty(attribute.getName())) {
+            addFailure(validationResult, attribute,
+                    "ObjAttribute name '%s' is invalid",
+                    attribute.getName());
+        }
+    }
 
-                        addFailure(
-                                validationResult,
-                                attribute,
-                                "EmbeddedAttribute '%s' has no DbAttribute mapping",
-                                attribute.getName());
-                    }
-                    else if (((ObjEntity) attribute.getEntity())
-                            .getDbEntity()
-                            .getAttribute(dbAttributeName) == null) {
-
-                        addFailure(
-                                validationResult,
-                                attribute,
-                                "EmbeddedAttribute '%s' has incorrect DbAttribute mapping",
-                                attribute.getName());
-                    }
-                }
-            }
+    private void validateDbAttribute(ObjAttribute attribute, ValidationResult validationResult)
{
+        if (attribute.getEntity().isAbstract()) {
+            // nothing to validate
+            // abstract entity does not have to define a dbAttribute
+            return;
+        }
 
+        DbAttribute dbAttribute;
+        try {
+            dbAttribute = attribute.getDbAttribute();
+        } catch (ExpressionException e) {
+            // see CAY-2153
+            // getDbAttribute() can fail if db path for this attribute is invalid
+            // so we catch it here and show nice validation failure instead of crash
+            addFailure(validationResult, attribute,
+                    "ObjAttribute '%s' has invalid DB path: %s",
+                    attribute.getName(),
+                    e.getExpressionString());
+            return;
         }
-        else if (attribute.getDbAttribute() == null) {
-            addFailure(
-                    validationResult,
-                    attribute,
+
+        if (dbAttribute == null) {
+            addFailure(validationResult, attribute,
                     "ObjAttribute '%s' has no DbAttribute mapping",
                     attribute.getName());
+            return;
         }
-        // can't support generated meaningful attributes for now; besides they don't make
-        // sense.
-        // TODO: andrus 03/10/2010 - is that really so? I think those are supported...
-        else if (attribute.getDbAttribute().isPrimaryKey()
-                && attribute.getDbAttribute().isGenerated()) {
-
-            addFailure(
-                    validationResult,
-                    attribute,
+
+        if (dbAttribute.isPrimaryKey() && dbAttribute.isGenerated()) {
+            // can't support generated meaningful attributes for now;
+            // besides they don't make sense.
+            // TODO: andrus 03/10/2010 - is that really so? I think those are supported...
+            addFailure(validationResult, attribute,
                     "ObjAttribute '%s' is mapped to a generated PK: %s",
                     attribute.getName(),
                     attribute.getDbAttributeName());
         }
+    }
 
-        checkForDuplicates(attribute, validationResult);
+    private void validateEmbeddable(EmbeddedAttribute attribute, ValidationResult validationResult)
{
+        Embeddable embeddable = attribute.getEmbeddable();
+
+        if (embeddable == null) {
+            String msg = attribute.getType() == null ?
+                    "EmbeddedAttribute '%s' has no Embeddable" :
+                    "EmbeddedAttribute '%s' has incorrect Embeddable";
+
+            addFailure(validationResult, attribute, msg, attribute.getName());
+            return;
+        }
+
+        Map<String, String> attrOverrides = attribute.getAttributeOverrides();
+
+        for (EmbeddableAttribute embeddableAttribute : embeddable.getAttributes()) {
+            String dbAttributeName;
+            if (!attrOverrides.isEmpty()
+                    && attrOverrides.containsKey(embeddableAttribute.getName()))
{
+                dbAttributeName = attrOverrides.get(embeddableAttribute.getName());
+            } else {
+                dbAttributeName = embeddableAttribute.getDbAttributeName();
+            }
+
+            if (Util.isEmptyString(dbAttributeName)) {
+                addFailure(validationResult, attribute,
+                        "EmbeddedAttribute '%s' has no DbAttribute mapping",
+                        attribute.getName());
+            } else if (attribute.getEntity()
+                    .getDbEntity()
+                    .getAttribute(dbAttributeName) == null) {
+                addFailure(validationResult, attribute,
+                        "EmbeddedAttribute '%s' has incorrect DbAttribute mapping",
+                        attribute.getName());
+            }
+        }
     }
 
     /**
@@ -158,11 +156,11 @@ class ObjAttributeValidator extends ConfigurationNodeValidator {
      */
     private void checkForDuplicates(ObjAttribute     attribute,
                                     ValidationResult validationResult) {
-        if (attribute               != null &&
-            attribute.getName()     != null &&
-            attribute.isInherited() == false) {
+        if ( attribute               != null &&
+             attribute.getName()     != null &&
+            !attribute.isInherited()) {
 
-            ObjEntity entity = (ObjEntity) attribute.getEntity();
+            ObjEntity entity = attribute.getEntity();
 
             for (ObjAttribute comparisonAttribute : entity.getAttributes()) {
                 if (attribute != comparisonAttribute) {
@@ -170,9 +168,7 @@ class ObjAttributeValidator extends ConfigurationNodeValidator {
 
                     if (dbAttributePath != null) {
                         if (dbAttributePath.equals(comparisonAttribute.getDbAttributePath()))
{
-                            addFailure
-                                (validationResult,
-                                 attribute,
+                            addFailure(validationResult, attribute,
                                  "ObjEntity '%s' contains a duplicate DbAttribute mapping
('%s' -> '%s')",
                                  entity.getName(),
                                  attribute.getName(),

http://git-wip-us.apache.org/repos/asf/cayenne/blob/bd522367/cayenne-server/src/main/java/org/apache/cayenne/map/ObjAttribute.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/map/ObjAttribute.java b/cayenne-server/src/main/java/org/apache/cayenne/map/ObjAttribute.java
index fa54af4..ea5b05e 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/map/ObjAttribute.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/map/ObjAttribute.java
@@ -114,8 +114,8 @@ public class ObjAttribute extends Attribute implements ConfigurationNode
{
         }
 
         // If this obj attribute is mapped to db attribute
-        if (getDbAttribute() != null
-                || (((ObjEntity) getEntity()).isAbstract() && !Util.isEmptyString(getDbAttributePath())))
{
+        if (/*getDbAttribute() != null
+                || (((ObjEntity) getEntity()).isAbstract() && */!Util.isEmptyString(getDbAttributePath()))
{
             encoder.print(" db-attribute-path=\"");
             encoder.print(Util.encodeXmlAttribute(getDbAttributePath()));
             encoder.print('\"');

http://git-wip-us.apache.org/repos/asf/cayenne/blob/bd522367/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 b6e2f8b..61a7bcd 100644
--- a/docs/doc/src/main/resources/RELEASE-NOTES.txt
+++ b/docs/doc/src/main/resources/RELEASE-NOTES.txt
@@ -69,6 +69,7 @@ CAY-2131 Modeler NullPointerException in reverse engineering when importing
diff
 CAY-2138 NVARCHAR, LONGNVARCHAR and NCLOB types are missing from Firebird types.xml
 CAY-2143 NPE in BaseSchemaUpdateStrategy
 CAY-2144 cdbimport always fails for databases which don't support catalogs
+CAY-2153 Modeler Exception in save action after reverse engineering some complex DB schema
 
 ----------------------------------
 Release: 4.0.M3

http://git-wip-us.apache.org/repos/asf/cayenne/blob/bd522367/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/dialog/db/DbLoaderHelper.java
----------------------------------------------------------------------
diff --git a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/dialog/db/DbLoaderHelper.java
b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/dialog/db/DbLoaderHelper.java
index b975dc6..d314214 100644
--- a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/dialog/db/DbLoaderHelper.java
+++ b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/dialog/db/DbLoaderHelper.java
@@ -49,6 +49,7 @@ import org.apache.cayenne.modeler.ProjectController;
 import org.apache.cayenne.modeler.dialog.DbImportProjectSaver;
 import org.apache.cayenne.modeler.pref.DBConnectionInfo;
 import org.apache.cayenne.modeler.util.LongRunningTask;
+import org.apache.cayenne.modeler.util.ProjectUtil;
 import org.apache.cayenne.tools.configuration.ToolsModule;
 import org.apache.cayenne.tools.dbimport.DbImportAction;
 import org.apache.cayenne.tools.dbimport.DbImportConfiguration;
@@ -373,6 +374,7 @@ public class DbLoaderHelper {
             } catch (Exception e) {
                 processException(e, "Error importing database schema.");
             }
+            ProjectUtil.cleanObjMappings(dataMap);
         }
 
         protected DbImportAction createAction(DataMap targetDataMap) {

http://git-wip-us.apache.org/repos/asf/cayenne/blob/bd522367/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/wrapper/ObjAttributeWrapper.java
----------------------------------------------------------------------
diff --git a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/wrapper/ObjAttributeWrapper.java
b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/wrapper/ObjAttributeWrapper.java
index 6e3cee2..bf5530a 100644
--- a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/wrapper/ObjAttributeWrapper.java
+++ b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/wrapper/ObjAttributeWrapper.java
@@ -20,6 +20,7 @@ package org.apache.cayenne.modeler.editor.wrapper;
 
 import java.util.List;
 
+import org.apache.cayenne.exp.ExpressionException;
 import org.apache.cayenne.map.DbAttribute;
 import org.apache.cayenne.map.Entity;
 import org.apache.cayenne.map.ObjAttribute;
@@ -139,7 +140,11 @@ public class ObjAttributeWrapper implements Wrapper<ObjAttribute>
{
     }
 
     public DbAttribute getDbAttribute() {
-        return objAttribute.getDbAttribute();
+        try {
+            return objAttribute.getDbAttribute();
+        } catch (ExpressionException e) {
+            return null;
+        }
     }
 
     public boolean isInherited() {


Mime
View raw message