This is an automated email from the ASF dual-hosted git repository. ntimofeev pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/cayenne.git
The following commit(s) were added to refs/heads/master by this push: new e60ac710b CAY-2838 Vertical Inheritance: Problem setting db attribute to null via flattened path e60ac710b is described below commit e60ac710be75799a735bbe1a458b4ba09704a5c7 Author: Nikita Timofeev <stari...@gmail.com> AuthorDate: Mon Mar 4 15:06:14 2024 +0400 CAY-2838 Vertical Inheritance: Problem setting db attribute to null via flattened path --- .../access/flush/ArcValuesCreationHandler.java | 19 +++++- .../cayenne/dba/sqlserver/SQLServerAdapter.java | 54 ++++++++++++++- .../cayenne/access/VerticalInheritanceIT.java | 78 ++++++++++++++-------- .../inheritance_vertical/auto/_IvAbstract.java | 19 ++++++ .../inheritance_vertical/auto/_IvConcrete.java | 17 +++++ .../test/resources/inheritance-vertical.map.xml | 9 +++ 6 files changed, 164 insertions(+), 32 deletions(-) diff --git a/cayenne/src/main/java/org/apache/cayenne/access/flush/ArcValuesCreationHandler.java b/cayenne/src/main/java/org/apache/cayenne/access/flush/ArcValuesCreationHandler.java index 545fd8e69..881e0de11 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/flush/ArcValuesCreationHandler.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/flush/ArcValuesCreationHandler.java @@ -114,7 +114,7 @@ class ArcValuesCreationHandler implements GraphChangeHandler { // intermediate db entity to be inserted DbEntity target = relationship.getTargetEntity(); // if ID is present, just use it, otherwise create new - // if this is last segment, and it's a relationship, use known target id from arc creation + // if this is the last segment, and it's a relationship, use known target id from arc creation if(!dbPathIterator.hasNext()) { targetId = finalTargetId; } else { @@ -146,8 +146,7 @@ class ArcValuesCreationHandler implements GraphChangeHandler { // should update existing DB row factory.getOrCreate(target, targetId, add ? DbRowOpType.UPDATE : defaultType); } - // should always add data from the intermediate relationship - processRelationship(relationship, srcId, targetId, dbPathIterator.hasNext() || add); + processRelationship(relationship, srcId, targetId, shouldProcessAsAddition(relationship, add)); srcId = targetId; // use target as next source } } @@ -155,6 +154,20 @@ class ArcValuesCreationHandler implements GraphChangeHandler { return targetId; } + private boolean shouldProcessAsAddition(DbRelationship relationship, boolean add) { + if(add) { + return true; + } + + // should always add data from one-to-one relationships + for(DbJoin join : relationship.getJoins()) { + if(!join.getSource().isPrimaryKey() || !join.getTarget().isPrimaryKey()) { + return false; + } + } + return true; + } + protected void processRelationship(DbRelationship dbRelationship, ObjectId srcId, ObjectId targetId, boolean add) { for(DbJoin join : dbRelationship.getJoins()) { boolean srcPK = join.getSource().isPrimaryKey(); diff --git a/cayenne/src/main/java/org/apache/cayenne/dba/sqlserver/SQLServerAdapter.java b/cayenne/src/main/java/org/apache/cayenne/dba/sqlserver/SQLServerAdapter.java index e8378ad55..9b532f560 100644 --- a/cayenne/src/main/java/org/apache/cayenne/dba/sqlserver/SQLServerAdapter.java +++ b/cayenne/src/main/java/org/apache/cayenne/dba/sqlserver/SQLServerAdapter.java @@ -20,8 +20,12 @@ package org.apache.cayenne.dba.sqlserver; import java.util.Arrays; +import java.util.Collection; +import java.util.Iterator; import java.util.List; +import java.util.stream.Collectors; +import org.apache.cayenne.CayenneRuntimeException; import org.apache.cayenne.access.DataNode; import org.apache.cayenne.access.sqlbuilder.sqltree.SQLTreeProcessor; import org.apache.cayenne.access.types.CharType; @@ -34,6 +38,8 @@ import org.apache.cayenne.configuration.Constants; import org.apache.cayenne.configuration.RuntimeProperties; import org.apache.cayenne.dba.sybase.SybaseAdapter; import org.apache.cayenne.di.Inject; +import org.apache.cayenne.map.DbAttribute; +import org.apache.cayenne.map.DbEntity; import org.apache.cayenne.query.Query; import org.apache.cayenne.query.SQLAction; import org.apache.cayenne.resource.ResourceLocator; @@ -111,7 +117,7 @@ public class SQLServerAdapter extends SybaseAdapter { public boolean supportsGeneratedKeysForBatchInserts() { return false; } - + /** * @since 4.2 */ @@ -163,4 +169,50 @@ public class SQLServerAdapter extends SybaseAdapter { public void setVersion(Integer version) { this.version = version; } + + /** + * Generates DDL to create unique index that allows multiple NULL values to comply with ANSI SQL, + * that is default behaviour for other RDBMS. + * <br> + * Example: + * <pre> + * {@code + * CREATE UNIQUE NONCLUSTERED INDEX _idx_entity_attribute + * ON entity(attribute) + * WHERE attribute IS NOT NULL + * } + * </pre> + * + * @param source entity for the index + * @param columns source columns for the index + * @return DDL to create unique index + * + * @since 4.2.1 + */ + @Override + public String createUniqueConstraint(DbEntity source, Collection<DbAttribute> columns) { + if (columns == null || columns.isEmpty()) { + throw new CayenneRuntimeException("Can't create UNIQUE constraint - no columns specified."); + } + + return "CREATE UNIQUE NONCLUSTERED INDEX " + uniqueIndexName(source, columns) + " ON " + + quotingStrategy.quotedFullyQualifiedName(source) + + "(" + + columns.stream().map(quotingStrategy::quotedName).collect(Collectors.joining(", ")) + + ") WHERE " + + columns.stream().map(quotingStrategy::quotedName) + .map(n -> n + " IS NOT NULL") + .collect(Collectors.joining(" AND ")); + } + + private String uniqueIndexName(DbEntity source, Collection<DbAttribute> columns) { + return "_idx_unique_" + + source.getName().replace(' ', '_').toLowerCase() + + "_" + + columns.stream() + .map(DbAttribute::getName) + .map(String::toLowerCase) + .map(n -> n.replace(' ', '_')) + .collect(Collectors.joining("_")); + } } diff --git a/cayenne/src/test/java/org/apache/cayenne/access/VerticalInheritanceIT.java b/cayenne/src/test/java/org/apache/cayenne/access/VerticalInheritanceIT.java index 7a3ae643f..32a7fed05 100644 --- a/cayenne/src/test/java/org/apache/cayenne/access/VerticalInheritanceIT.java +++ b/cayenne/src/test/java/org/apache/cayenne/access/VerticalInheritanceIT.java @@ -32,6 +32,8 @@ import org.apache.cayenne.testdo.inheritance_vertical.*; import org.apache.cayenne.unit.di.runtime.CayenneProjects; import org.apache.cayenne.unit.di.runtime.RuntimeCase; import org.apache.cayenne.unit.di.runtime.UseCayenneRuntime; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import java.sql.SQLException; @@ -56,6 +58,29 @@ public class VerticalInheritanceIT extends RuntimeCase { @Inject protected CayenneRuntime runtime; + TableHelper ivAbstractTable; + + TableHelper ivConcreteTable; + + @Before + public void setup() { + ivAbstractTable = new TableHelper(dbHelper, "IV_ABSTRACT"); + ivAbstractTable.setColumns("ID", "PARENT_ID", "TYPE") + .setColumnTypes(Types.INTEGER, Types.INTEGER, Types.CHAR); + ivConcreteTable = new TableHelper(dbHelper, "IV_CONCRETE"); + ivConcreteTable.setColumns("ID", "NAME", "RELATED_ABSTRACT_ID") + .setColumnTypes(Types.INTEGER, Types.VARCHAR, Types.INTEGER); + } + + @After + public void cleanUpConcrete() throws SQLException { + ivConcreteTable.deleteAll(); + ivAbstractTable.deleteAll(); + + assertEquals(0, ivAbstractTable.getRowCount()); + assertEquals(0, ivConcreteTable.getRowCount()); + } + @Test public void testInsert_Root() throws Exception { @@ -593,7 +618,7 @@ public class VerticalInheritanceIT extends RuntimeCase { } @Test - public void testUpdateWithRelationship() { + public void testUpdateWithRelationship() throws SQLException { IvConcrete parent1 = context.newObject(IvConcrete.class); parent1.setName("Parent1"); context.commitChanges(); @@ -621,7 +646,7 @@ public class VerticalInheritanceIT extends RuntimeCase { * @link https://issues.apache.org/jira/browse/CAY-2838 */ @Test - public void testNullifyFlattenedAttribute() { + public void testNullifyFlattenedAttribute() throws SQLException { IvConcrete concrete = context.newObject(IvConcrete.class); concrete.setName("Concrete"); context.commitChanges(); @@ -665,14 +690,6 @@ public class VerticalInheritanceIT extends RuntimeCase { @Test public void testDeleteFlattenedNoValues() throws SQLException { - TableHelper ivAbstractTable = new TableHelper(dbHelper, "IV_ABSTRACT"); - ivAbstractTable.setColumns("ID", "PARENT_ID", "TYPE") - .setColumnTypes(Types.INTEGER, Types.INTEGER, Types.CHAR); - - TableHelper ivConcreteTable = new TableHelper(dbHelper, "IV_CONCRETE"); - ivConcreteTable.setColumns("ID", "NAME") - .setColumnTypes(Types.INTEGER, Types.VARCHAR); - ivAbstractTable.insert(1, null, "S"); IvConcrete concrete = SelectById.query(IvConcrete.class, 1).selectOne(context); @@ -688,16 +705,8 @@ public class VerticalInheritanceIT extends RuntimeCase { @Test public void testDeleteFlattenedNullValues() throws SQLException { - TableHelper ivAbstractTable = new TableHelper(dbHelper, "IV_ABSTRACT"); - ivAbstractTable.setColumns("ID", "PARENT_ID", "TYPE") - .setColumnTypes(Types.INTEGER, Types.INTEGER, Types.CHAR); - - TableHelper ivConcreteTable = new TableHelper(dbHelper, "IV_CONCRETE"); - ivConcreteTable.setColumns("ID", "NAME") - .setColumnTypes(Types.INTEGER, Types.VARCHAR); - ivAbstractTable.insert(1, null, "S"); - ivConcreteTable.insert(1, null); + ivConcreteTable.insert(1, null, null); IvConcrete concrete = SelectById.query(IvConcrete.class, 1).selectOne(context); assertNotNull(concrete); @@ -712,16 +721,8 @@ public class VerticalInheritanceIT extends RuntimeCase { @Test public void testDeleteFlattenedNullifyValues() throws SQLException { - TableHelper ivAbstractTable = new TableHelper(dbHelper, "IV_ABSTRACT"); - ivAbstractTable.setColumns("ID", "PARENT_ID", "TYPE") - .setColumnTypes(Types.INTEGER, Types.INTEGER, Types.CHAR); - - TableHelper ivConcreteTable = new TableHelper(dbHelper, "IV_CONCRETE"); - ivConcreteTable.setColumns("ID", "NAME") - .setColumnTypes(Types.INTEGER, Types.VARCHAR); - ivAbstractTable.insert(1, null, "S"); - ivConcreteTable.insert(1, "test"); + ivConcreteTable.insert(1, "test", null); IvConcrete concrete = SelectById.query(IvConcrete.class, 1).selectOne(context); assertNotNull(concrete); @@ -741,6 +742,27 @@ public class VerticalInheritanceIT extends RuntimeCase { assertEquals(0, ivConcreteTable.getRowCount()); } + @Test + public void testNullifyFlattenedRelationshipConcreteToAbstract() throws SQLException { + ivAbstractTable.insert(1, null, "S"); + ivConcreteTable.insert(1, "One", null); + ivAbstractTable.insert(2, null, "S"); + ivConcreteTable.insert(2, "Two", 1); + + IvConcrete concrete = SelectById.query(IvConcrete.class, 2).selectOne(context); + concrete.setRelatedAbstract(null); + + context.commitChanges(); + assertNull(concrete.getRelatedAbstract()); + + { + ObjectContext cleanContext = runtime.newContext(); + IvConcrete concreteFetched = SelectById.query(IvConcrete.class, 2).selectOne(cleanContext); + assertEquals("Two", concreteFetched.getName()); + assertNull(concreteFetched.getRelatedAbstract()); + } + } + @Test//(expected = ValidationException.class) // other2 is not mandatory for now public void testInsertWithAttributeAndRelationship() { IvOther other = context.newObject(IvOther.class); diff --git a/cayenne/src/test/java/org/apache/cayenne/testdo/inheritance_vertical/auto/_IvAbstract.java b/cayenne/src/test/java/org/apache/cayenne/testdo/inheritance_vertical/auto/_IvAbstract.java index f2b6c6ecb..df9d53de4 100644 --- a/cayenne/src/test/java/org/apache/cayenne/testdo/inheritance_vertical/auto/_IvAbstract.java +++ b/cayenne/src/test/java/org/apache/cayenne/testdo/inheritance_vertical/auto/_IvAbstract.java @@ -5,11 +5,13 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import org.apache.cayenne.PersistentObject; +import org.apache.cayenne.exp.property.EntityProperty; import org.apache.cayenne.exp.property.NumericIdProperty; import org.apache.cayenne.exp.property.PropertyFactory; import org.apache.cayenne.exp.property.SelfProperty; import org.apache.cayenne.exp.property.StringProperty; import org.apache.cayenne.testdo.inheritance_vertical.IvAbstract; +import org.apache.cayenne.testdo.inheritance_vertical.IvConcrete; /** * Class _IvAbstract was generated by Cayenne. @@ -27,9 +29,11 @@ public abstract class _IvAbstract extends PersistentObject { public static final String ID_PK_COLUMN = "ID"; public static final StringProperty<String> TYPE = PropertyFactory.createString("type", String.class); + public static final EntityProperty<IvConcrete> RELATED_CONCRETE = PropertyFactory.createEntity("relatedConcrete", IvConcrete.class); protected String type; + protected Object relatedConcrete; public void setType(String type) { beforePropertyWrite("type", this.type, type); @@ -41,6 +45,14 @@ public abstract class _IvAbstract extends PersistentObject { return this.type; } + public void setRelatedConcrete(IvConcrete relatedConcrete) { + setToOneTarget("relatedConcrete", relatedConcrete, true); + } + + public IvConcrete getRelatedConcrete() { + return (IvConcrete)readProperty("relatedConcrete"); + } + @Override public Object readPropertyDirectly(String propName) { if(propName == null) { @@ -50,6 +62,8 @@ public abstract class _IvAbstract extends PersistentObject { switch(propName) { case "type": return this.type; + case "relatedConcrete": + return this.relatedConcrete; default: return super.readPropertyDirectly(propName); } @@ -65,6 +79,9 @@ public abstract class _IvAbstract extends PersistentObject { case "type": this.type = (String)val; break; + case "relatedConcrete": + this.relatedConcrete = val; + break; default: super.writePropertyDirectly(propName, val); } @@ -82,12 +99,14 @@ public abstract class _IvAbstract extends PersistentObject { protected void writeState(ObjectOutputStream out) throws IOException { super.writeState(out); out.writeObject(this.type); + out.writeObject(this.relatedConcrete); } @Override protected void readState(ObjectInputStream in) throws IOException, ClassNotFoundException { super.readState(in); this.type = (String)in.readObject(); + this.relatedConcrete = in.readObject(); } } diff --git a/cayenne/src/test/java/org/apache/cayenne/testdo/inheritance_vertical/auto/_IvConcrete.java b/cayenne/src/test/java/org/apache/cayenne/testdo/inheritance_vertical/auto/_IvConcrete.java index edb4676ab..a83018692 100644 --- a/cayenne/src/test/java/org/apache/cayenne/testdo/inheritance_vertical/auto/_IvConcrete.java +++ b/cayenne/src/test/java/org/apache/cayenne/testdo/inheritance_vertical/auto/_IvConcrete.java @@ -32,11 +32,13 @@ public abstract class _IvConcrete extends IvAbstract { public static final StringProperty<String> NAME = PropertyFactory.createString("name", String.class); public static final ListProperty<IvConcrete> CHILDREN = PropertyFactory.createList("children", IvConcrete.class); public static final EntityProperty<IvConcrete> PARENT = PropertyFactory.createEntity("parent", IvConcrete.class); + public static final EntityProperty<IvAbstract> RELATED_ABSTRACT = PropertyFactory.createEntity("relatedAbstract", IvAbstract.class); protected String name; protected Object children; protected Object parent; + protected Object relatedAbstract; public void setName(String name) { beforePropertyWrite("name", this.name, name); @@ -69,6 +71,14 @@ public abstract class _IvConcrete extends IvAbstract { return (IvConcrete)readProperty("parent"); } + public void setRelatedAbstract(IvAbstract relatedAbstract) { + setToOneTarget("relatedAbstract", relatedAbstract, true); + } + + public IvAbstract getRelatedAbstract() { + return (IvAbstract)readProperty("relatedAbstract"); + } + @Override public Object readPropertyDirectly(String propName) { if(propName == null) { @@ -82,6 +92,8 @@ public abstract class _IvConcrete extends IvAbstract { return this.children; case "parent": return this.parent; + case "relatedAbstract": + return this.relatedAbstract; default: return super.readPropertyDirectly(propName); } @@ -103,6 +115,9 @@ public abstract class _IvConcrete extends IvAbstract { case "parent": this.parent = val; break; + case "relatedAbstract": + this.relatedAbstract = val; + break; default: super.writePropertyDirectly(propName, val); } @@ -122,6 +137,7 @@ public abstract class _IvConcrete extends IvAbstract { out.writeObject(this.name); out.writeObject(this.children); out.writeObject(this.parent); + out.writeObject(this.relatedAbstract); } @Override @@ -130,6 +146,7 @@ public abstract class _IvConcrete extends IvAbstract { this.name = (String)in.readObject(); this.children = in.readObject(); this.parent = in.readObject(); + this.relatedAbstract = in.readObject(); } } diff --git a/cayenne/src/test/resources/inheritance-vertical.map.xml b/cayenne/src/test/resources/inheritance-vertical.map.xml index 8df641339..ff892937b 100644 --- a/cayenne/src/test/resources/inheritance-vertical.map.xml +++ b/cayenne/src/test/resources/inheritance-vertical.map.xml @@ -42,6 +42,7 @@ <db-entity name="IV_CONCRETE"> <db-attribute name="ID" type="INTEGER" isPrimaryKey="true" isMandatory="true"/> <db-attribute name="NAME" type="VARCHAR" length="100"/> + <db-attribute name="RELATED_ABSTRACT_ID" type="INTEGER"/> </db-entity> <db-entity name="IV_GEN_KEY_ROOT"> <db-attribute name="DISCRIMINATOR" type="VARCHAR" length="10"/> @@ -200,6 +201,9 @@ <db-relationship name="parent" source="IV_ABSTRACT" target="IV_ABSTRACT"> <db-attribute-pair source="PARENT_ID" target="ID"/> </db-relationship> + <db-relationship name="relatedConcrete" source="IV_ABSTRACT" target="IV_CONCRETE"> + <db-attribute-pair source="ID" target="RELATED_ABSTRACT_ID"/> + </db-relationship> <db-relationship name="impl" source="IV_BASE" target="IV_IMPL" toDependentPK="true"> <db-attribute-pair source="ID" target="ID"/> </db-relationship> @@ -212,6 +216,9 @@ <db-relationship name="abstract" source="IV_CONCRETE" target="IV_ABSTRACT"> <db-attribute-pair source="ID" target="ID"/> </db-relationship> + <db-relationship name="relatedAbstract" source="IV_CONCRETE" target="IV_ABSTRACT"> + <db-attribute-pair source="RELATED_ABSTRACT_ID" target="ID"/> + </db-relationship> <db-relationship name="sub1" source="IV_GEN_KEY_ROOT" target="IV_GEN_KEY_SUB" toDependentPK="true"> <db-attribute-pair source="ID" target="ID"/> </db-relationship> @@ -270,8 +277,10 @@ <db-attribute-pair source="IV_ROOT_ID" target="ID"/> </db-relationship> <obj-relationship name="x" source="Iv2Sub1" target="Iv2X" deleteRule="Nullify" db-relationship-path="sub1.x"/> + <obj-relationship name="relatedConcrete" source="IvAbstract" target="IvConcrete" deleteRule="Nullify" db-relationship-path="relatedConcrete.abstract"/> <obj-relationship name="others" source="IvBase" target="IvOther" deleteRule="Deny" db-relationship-path="others"/> <obj-relationship name="children" source="IvConcrete" target="IvConcrete" deleteRule="Deny" db-relationship-path="children"/> + <obj-relationship name="relatedAbstract" source="IvConcrete" target="IvAbstract" deleteRule="Nullify" db-relationship-path="concrete.relatedAbstract"/> <obj-relationship name="parent" source="IvConcrete" target="IvConcrete" deleteRule="Nullify" db-relationship-path="parent"/> <obj-relationship name="other1" source="IvImpl" target="IvOther" deleteRule="Nullify" db-relationship-path="impl.other1"/> <obj-relationship name="other2" source="IvImpl" target="IvOther" deleteRule="Nullify" db-relationship-path="impl.other2"/>