From a407560ffd99fca18f0efe1f26117abe2ab09310 Mon Sep 17 00:00:00 2001 From: Eduardo Perez Ureta Date: Tue, 17 Sep 2019 05:14:14 +0000 Subject: [PATCH] [master] Bug 551815: When an entity with an aggregate with all fields set to null is changed to null for the aggregate field then Eclipselink executes a bogus no-op SQL update statement Signed-off-by: Eduardo Perez Ureta --- .../models/aggregate/SimpleAggregate.java | 24 +++++ .../aggregate/SimpleAggregateProject.java | 95 +++++++++++++++++++ .../aggregate/SimpleAggregateSystem.java | 87 +++++++++++++++++ .../models/aggregate/SimpleEntity.java | 56 +++++++++++ .../aggregate/SimpleAggregateTestModel.java | 49 ++++++++++ .../internal/descriptors/ObjectBuilder.java | 18 ++++ .../mappings/AggregateMapping.java | 15 ++- 7 files changed, 342 insertions(+), 2 deletions(-) create mode 100644 foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregate.java create mode 100644 foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateProject.java create mode 100644 foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateSystem.java create mode 100644 foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleEntity.java create mode 100644 foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/aggregate/SimpleAggregateTestModel.java diff --git a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregate.java b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregate.java new file mode 100644 index 00000000000..10c38cdb3ba --- /dev/null +++ b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregate.java @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, + * or the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause + */ +package org.eclipse.persistence.testing.models.aggregate; + +public class SimpleAggregate { + private String content; + + public String getContent() { + return content; + } + + public void setContent(String content) { + this.content = content; + } +} diff --git a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateProject.java b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateProject.java new file mode 100644 index 00000000000..83cd59db3af --- /dev/null +++ b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateProject.java @@ -0,0 +1,95 @@ +/* + * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, + * or the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause + */ +package org.eclipse.persistence.testing.models.aggregate; + +import org.eclipse.persistence.descriptors.ChangedFieldsLockingPolicy; +import org.eclipse.persistence.descriptors.ClassDescriptor; +import org.eclipse.persistence.descriptors.RelationalDescriptor; +import org.eclipse.persistence.mappings.AggregateObjectMapping; +import org.eclipse.persistence.mappings.DirectToFieldMapping; + +public class SimpleAggregateProject extends org.eclipse.persistence.sessions.Project { + + public SimpleAggregateProject() { + applyPROJECT(); + applyLOGIN(); + + addDescriptor(buildSimpleAggregateClassDescriptor()); + addDescriptor(buildSimpleEntityDescriptor()); + } + + protected void applyLOGIN() { + } + + protected void applyPROJECT() { + setName("SimpleAggregateSystem"); + } + + public ClassDescriptor buildSimpleAggregateClassDescriptor() { + RelationalDescriptor descriptor = new RelationalDescriptor(); + descriptor.descriptorIsAggregate(); + descriptor.setJavaClass(SimpleAggregate.class); + + // ClassDescriptor Properties. + descriptor.setAlias("SimpleAggregate"); + + + // Query Manager. + + + // Event Manager. + + // Mappings. + DirectToFieldMapping contentMapping = new DirectToFieldMapping(); + contentMapping.setAttributeName("content"); + contentMapping.setFieldName("content->DIRECT"); + descriptor.addMapping(contentMapping); + + return descriptor; + } + + protected ClassDescriptor buildSimpleEntityDescriptor() { + RelationalDescriptor descriptor = new RelationalDescriptor(); + //descriptor.setCacheable(false); + + // SECTION: DESCRIPTOR + descriptor.setJavaClass(SimpleEntity.class); + descriptor.addTableName("SIMPLEENTITY"); + descriptor.addPrimaryKeyFieldName("SIMPLEENTITY.ID"); + + ChangedFieldsLockingPolicy lockingPolicy = new ChangedFieldsLockingPolicy(); + descriptor.setOptimisticLockingPolicy(lockingPolicy); + + // SECTION: DIRECTTOFIELDMAPPING + DirectToFieldMapping directtofieldmapping = new DirectToFieldMapping(); + directtofieldmapping.setAttributeName("id"); + directtofieldmapping.setIsReadOnly(false); + directtofieldmapping.setFieldName("SIMPLEENTITY.ID"); + descriptor.addMapping(directtofieldmapping); + + DirectToFieldMapping fieldmapping = new DirectToFieldMapping(); + fieldmapping.setAttributeName("field"); + fieldmapping.setIsReadOnly(false); + fieldmapping.setFieldName("SIMPLEENTITY.FIELD"); + descriptor.addMapping(fieldmapping); + + AggregateObjectMapping aggregateObjectMapping = new AggregateObjectMapping(); + aggregateObjectMapping.setAttributeName("simpleAggregate"); + aggregateObjectMapping.setReferenceClass(SimpleAggregate.class); + aggregateObjectMapping.setIsNullAllowed(true); + aggregateObjectMapping.addFieldNameTranslation("SIMPLEENTITY.CONTENT", "content->DIRECT"); + descriptor.addMapping(aggregateObjectMapping); + + return descriptor; + } + +} diff --git a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateSystem.java b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateSystem.java new file mode 100644 index 00000000000..6c8353f41ee --- /dev/null +++ b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateSystem.java @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, + * or the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause + */ +package org.eclipse.persistence.testing.models.aggregate; + +import org.eclipse.persistence.queries.ReadObjectQuery; +import org.eclipse.persistence.sessions.DatabaseSession; +import org.eclipse.persistence.sessions.UnitOfWork; +import org.eclipse.persistence.testing.framework.TestSystem; +import org.eclipse.persistence.testing.tests.dynamic.QuerySQLTracker; +import org.eclipse.persistence.tools.schemaframework.SchemaManager; + +public class SimpleAggregateSystem extends TestSystem { + + public SimpleAggregateSystem() { + project = new SimpleAggregateProject(); + } + + @Override + public void addDescriptors(DatabaseSession session) { + if (project == null) { + project = new SimpleAggregateProject(); + } + session.addDescriptors(project); + } + + @Override + public void createTables(DatabaseSession session) { + SchemaManager schemaManager = new SchemaManager(session); + + schemaManager.replaceObject(SimpleEntity.tableDefinition()); + } + + @Override + public void populate(DatabaseSession session) { + + QuerySQLTracker qTracker = QuerySQLTracker.install(session); + + { + UnitOfWork uow = session.acquireUnitOfWork(); + + SimpleEntity instance = new SimpleEntity(); + instance.setId("1"); + instance.setField("constant"); + instance.setSimpleAggregate(new SimpleAggregate()); + session.writeObject(instance); + uow.registerObject(instance); + uow.commit(); + } + + { + UnitOfWork uow = session.acquireUnitOfWork(); + + ReadObjectQuery query = new ReadObjectQuery(); + final SimpleEntity queryInstance = new SimpleEntity(); + queryInstance.setId("1"); + query.setSelectionObject(queryInstance); + final SimpleEntity instance = (SimpleEntity) uow.executeQuery(query); + if (instance == null) { + throw new RuntimeException("Object was not found"); + } + if (instance.getSimpleAggregate() == null) { + //throw new RuntimeException("SimpleAggregate is null"); + } + instance.setSimpleAggregate(instance.getSimpleAggregate() == null ? new SimpleAggregate() : null); + + qTracker.reset(); + + instance.setSimpleAggregate(null); + uow.registerObject(instance); + uow.commit(); + + if (qTracker.getQueries().size() > 0) { + throw new RuntimeException("Unexpected query was executed: " + qTracker.getQueries()); + } + } + + } +} diff --git a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleEntity.java b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleEntity.java new file mode 100644 index 00000000000..43aef9b4897 --- /dev/null +++ b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleEntity.java @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, + * or the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause + */ +package org.eclipse.persistence.testing.models.aggregate; + +import org.eclipse.persistence.tools.schemaframework.TableDefinition; + +public class SimpleEntity { + private String id; + private String field; + private SimpleAggregate simpleAggregate; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public String getField() { + return field; + } + + public void setField(String field) { + this.field = field; + } + + public SimpleAggregate getSimpleAggregate() { + return simpleAggregate; + } + + public void setSimpleAggregate(SimpleAggregate simpleAggregate) { + this.simpleAggregate = simpleAggregate; + } + + public static TableDefinition tableDefinition() { + TableDefinition definition = new TableDefinition(); + + definition.setName("SIMPLEENTITY"); + + definition.addIdentityField("ID", String.class, 15); + definition.addField("FIELD", String.class, 15); + definition.addField("CONTENT", String.class, 20); + + return definition; + } +} diff --git a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/aggregate/SimpleAggregateTestModel.java b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/aggregate/SimpleAggregateTestModel.java new file mode 100644 index 00000000000..e522cfad21a --- /dev/null +++ b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/aggregate/SimpleAggregateTestModel.java @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, + * or the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause + */ +package org.eclipse.persistence.testing.tests.aggregate; + +import org.eclipse.persistence.testing.framework.TestModel; +import org.eclipse.persistence.testing.framework.TestSuite; +import org.eclipse.persistence.testing.models.aggregate.SimpleAggregateSystem; + +public class SimpleAggregateTestModel extends TestModel { + public SimpleAggregateTestModel() { + setDescription("This model tests reading/writing/deleting of the simple aggregate model."); + } + + @Override + public void addForcedRequiredSystems() { + //We need to ensure that the correct database schema is created + addForcedRequiredSystem(new SimpleAggregateSystem()); + } + + @Override + public void addRequiredSystems() { + } + + @Override + public void addTests() { + addTest(getUpdateObjectTestSuite()); + } + + public static TestSuite getUpdateObjectTestSuite() { + TestSuite suite = new TestSuite(); + suite.setName("AggregateUpdateObjectTestSuite"); + suite.setDescription("This suite tests the updating of each object in the aggregate model."); + + return suite; + } + + public static junit.framework.TestSuite suite() { + return new SimpleAggregateTestModel(); + } +} diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/descriptors/ObjectBuilder.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/descriptors/ObjectBuilder.java index 6e7a44e1394..9f8bd61cb27 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/descriptors/ObjectBuilder.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/descriptors/ObjectBuilder.java @@ -4639,4 +4639,22 @@ public boolean hasCacheIndexesInSopObject() { public AbstractRecord createRecordFromXMLContext(XMLContext context) { return createRecord((AbstractSession)context.getSession()); } + + public boolean checkNull(Object o, AbstractSession session) { + if (o == null) { + return true; + } + // PERF: Avoid iterator. + final List mappings = this.descriptor.getMappings(); + for (int index = 0; index < mappings.size(); index++) { + final DatabaseMapping mapping = (DatabaseMapping)mappings.get(index); + + final Object value = mapping.getAttributeValueFromObject(o); + if (value != null) { + return false; + } + } + + return true; + } } diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/mappings/AggregateMapping.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/mappings/AggregateMapping.java index 378b1b00e7b..1431ea6851f 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/mappings/AggregateMapping.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/mappings/AggregateMapping.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2018 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2019 Oracle and/or its affiliates. All rights reserved. * Copyright (c) 1998, 2018 IBM Corporation. All rights reserved. * * This program and the accompanying materials are made available under the @@ -36,6 +36,7 @@ import org.eclipse.persistence.exceptions.*; import org.eclipse.persistence.expressions.*; import org.eclipse.persistence.internal.descriptors.*; +import org.eclipse.persistence.internal.helper.DatabaseField; import org.eclipse.persistence.internal.helper.IdentityHashSet; import org.eclipse.persistence.internal.identitymaps.CacheKey; import org.eclipse.persistence.internal.security.PrivilegedAccessHelper; @@ -316,7 +317,9 @@ public ChangeRecord compareForChange(Object clone, Object backup, ObjectChangeSe if (!owner.isNew()) { backupAttribute = getAttributeValueFromObject(backup); - if ((cloneAttribute == null) && (backupAttribute == null)) { + if ((cloneAttribute == null || backupAttribute == null) && + checkNull(cloneAttribute, session) && checkNull(backupAttribute, session) + ) { return null;// no change } if ((cloneAttribute != null) && (backupAttribute != null) && (!cloneAttribute.getClass().equals(backupAttribute.getClass()))) { @@ -354,6 +357,14 @@ public ChangeRecord compareForChange(Object clone, Object backup, ObjectChangeSe return changeRecord; } + public boolean checkNull(final Object o, final AbstractSession session) { + if (o == null) { + return true; + } + final ObjectBuilder builder = getObjectBuilder(o, session); + return builder.checkNull(o, session); + } + /** * INTERNAL: * Compare the attributes belonging to this mapping for the objects.