Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id A8A62200BCB for ; Thu, 24 Nov 2016 15:54:52 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id A70CB160B1E; Thu, 24 Nov 2016 14:54:52 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 7E4C7160B11 for ; Thu, 24 Nov 2016 15:54:51 +0100 (CET) Received: (qmail 20856 invoked by uid 500); 24 Nov 2016 14:54:50 -0000 Mailing-List: contact commits-help@cayenne.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cayenne.apache.org Delivered-To: mailing list commits@cayenne.apache.org Received: (qmail 20844 invoked by uid 99); 24 Nov 2016 14:54:50 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 24 Nov 2016 14:54:50 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 8C138DFCC7; Thu, 24 Nov 2016 14:54:50 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: skolbachev@apache.org To: commits@cayenne.apache.org Date: Thu, 24 Nov 2016 14:54:50 -0000 Message-Id: <740446ea30084b9794df0ced864fe24a@git.apache.org> X-Mailer: ASF-Git Admin Mailer 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 archived-at: Thu, 24 Nov 2016 14:54:52 -0000 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 Authored: Thu Nov 24 14:24:57 2016 +0300 Committer: Nikita Timofeev 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 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 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 { } public DbAttribute getDbAttribute() { - return objAttribute.getDbAttribute(); + try { + return objAttribute.getDbAttribute(); + } catch (ExpressionException e) { + return null; + } } public boolean isInherited() {