Skip to content

Commit 194db07

Browse files
eperezeperez
committed
[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
1 parent 2637c10 commit 194db07

File tree

7 files changed

+341
-1
lines changed

7 files changed

+341
-1
lines changed

‎foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregate.java

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright (c) 1998, 2018 Oracle and/or its affiliates. All rights reserved.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0,
7+
* or the Eclipse Distribution License v. 1.0 which is available at
8+
* http://www.eclipse.org/org/documents/edl-v10.php.
9+
*
10+
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
11+
*/
12+
package org.eclipse.persistence.testing.models.aggregate;
13+
14+
public class SimpleAggregate {
15+
private String content;
16+
17+
public String getContent() {
18+
return content;
19+
}
20+
21+
public void setContent(String content) {
22+
this.content = content;
23+
}
24+
}

‎foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateProject.java

+95
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Copyright (c) 1998, 2018 Oracle and/or its affiliates. All rights reserved.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0,
7+
* or the Eclipse Distribution License v. 1.0 which is available at
8+
* http://www.eclipse.org/org/documents/edl-v10.php.
9+
*
10+
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
11+
*/
12+
package org.eclipse.persistence.testing.models.aggregate;
13+
14+
import org.eclipse.persistence.descriptors.ChangedFieldsLockingPolicy;
15+
import org.eclipse.persistence.descriptors.ClassDescriptor;
16+
import org.eclipse.persistence.descriptors.RelationalDescriptor;
17+
import org.eclipse.persistence.mappings.AggregateObjectMapping;
18+
import org.eclipse.persistence.mappings.DirectToFieldMapping;
19+
20+
public class SimpleAggregateProject extends org.eclipse.persistence.sessions.Project {
21+
22+
public SimpleAggregateProject() {
23+
applyPROJECT();
24+
applyLOGIN();
25+
26+
addDescriptor(buildSimpleAggregateClassDescriptor());
27+
addDescriptor(buildSimpleEntityDescriptor());
28+
}
29+
30+
protected void applyLOGIN() {
31+
}
32+
33+
protected void applyPROJECT() {
34+
setName("SimpleAggregateSystem");
35+
}
36+
37+
public ClassDescriptor buildSimpleAggregateClassDescriptor() {
38+
RelationalDescriptor descriptor = new RelationalDescriptor();
39+
descriptor.descriptorIsAggregate();
40+
descriptor.setJavaClass(SimpleAggregate.class);
41+
42+
// ClassDescriptor Properties.
43+
descriptor.setAlias("SimpleAggregate");
44+
45+
46+
// Query Manager.
47+
48+
49+
// Event Manager.
50+
51+
// Mappings.
52+
DirectToFieldMapping contentMapping = new DirectToFieldMapping();
53+
contentMapping.setAttributeName("content");
54+
contentMapping.setFieldName("content->DIRECT");
55+
descriptor.addMapping(contentMapping);
56+
57+
return descriptor;
58+
}
59+
60+
protected ClassDescriptor buildSimpleEntityDescriptor() {
61+
RelationalDescriptor descriptor = new RelationalDescriptor();
62+
//descriptor.setCacheable(false);
63+
64+
// SECTION: DESCRIPTOR
65+
descriptor.setJavaClass(SimpleEntity.class);
66+
descriptor.addTableName("SIMPLEENTITY");
67+
descriptor.addPrimaryKeyFieldName("SIMPLEENTITY.ID");
68+
69+
ChangedFieldsLockingPolicy lockingPolicy = new ChangedFieldsLockingPolicy();
70+
descriptor.setOptimisticLockingPolicy(lockingPolicy);
71+
72+
// SECTION: DIRECTTOFIELDMAPPING
73+
DirectToFieldMapping directtofieldmapping = new DirectToFieldMapping();
74+
directtofieldmapping.setAttributeName("id");
75+
directtofieldmapping.setIsReadOnly(false);
76+
directtofieldmapping.setFieldName("SIMPLEENTITY.ID");
77+
descriptor.addMapping(directtofieldmapping);
78+
79+
DirectToFieldMapping fieldmapping = new DirectToFieldMapping();
80+
fieldmapping.setAttributeName("field");
81+
fieldmapping.setIsReadOnly(false);
82+
fieldmapping.setFieldName("SIMPLEENTITY.FIELD");
83+
descriptor.addMapping(fieldmapping);
84+
85+
AggregateObjectMapping aggregateObjectMapping = new AggregateObjectMapping();
86+
aggregateObjectMapping.setAttributeName("simpleAggregate");
87+
aggregateObjectMapping.setReferenceClass(SimpleAggregate.class);
88+
aggregateObjectMapping.setIsNullAllowed(true);
89+
aggregateObjectMapping.addFieldNameTranslation("SIMPLEENTITY.CONTENT", "content->DIRECT");
90+
descriptor.addMapping(aggregateObjectMapping);
91+
92+
return descriptor;
93+
}
94+
95+
}

‎foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleAggregateSystem.java

+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright (c) 1998, 2018 Oracle and/or its affiliates. All rights reserved.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0,
7+
* or the Eclipse Distribution License v. 1.0 which is available at
8+
* http://www.eclipse.org/org/documents/edl-v10.php.
9+
*
10+
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
11+
*/
12+
package org.eclipse.persistence.testing.models.aggregate;
13+
14+
import org.eclipse.persistence.queries.ReadObjectQuery;
15+
import org.eclipse.persistence.sessions.DatabaseSession;
16+
import org.eclipse.persistence.sessions.UnitOfWork;
17+
import org.eclipse.persistence.testing.framework.TestSystem;
18+
import org.eclipse.persistence.testing.tests.dynamic.QuerySQLTracker;
19+
import org.eclipse.persistence.tools.schemaframework.SchemaManager;
20+
21+
public class SimpleAggregateSystem extends TestSystem {
22+
23+
public SimpleAggregateSystem() {
24+
project = new SimpleAggregateProject();
25+
}
26+
27+
@Override
28+
public void addDescriptors(DatabaseSession session) {
29+
if (project == null) {
30+
project = new SimpleAggregateProject();
31+
}
32+
session.addDescriptors(project);
33+
}
34+
35+
@Override
36+
public void createTables(DatabaseSession session) {
37+
SchemaManager schemaManager = new SchemaManager(session);
38+
39+
schemaManager.replaceObject(SimpleEntity.tableDefinition());
40+
}
41+
42+
@Override
43+
public void populate(DatabaseSession session) {
44+
45+
QuerySQLTracker qTracker = QuerySQLTracker.install(session);
46+
47+
{
48+
UnitOfWork uow = session.acquireUnitOfWork();
49+
50+
SimpleEntity instance = new SimpleEntity();
51+
instance.setId("1");
52+
instance.setField("constant");
53+
instance.setSimpleAggregate(new SimpleAggregate());
54+
session.writeObject(instance);
55+
uow.registerObject(instance);
56+
uow.commit();
57+
}
58+
59+
{
60+
UnitOfWork uow = session.acquireUnitOfWork();
61+
62+
ReadObjectQuery query = new ReadObjectQuery();
63+
final SimpleEntity queryInstance = new SimpleEntity();
64+
queryInstance.setId("1");
65+
query.setSelectionObject(queryInstance);
66+
final SimpleEntity instance = (SimpleEntity) uow.executeQuery(query);
67+
if (instance == null) {
68+
throw new RuntimeException("Object was not found");
69+
}
70+
if (instance.getSimpleAggregate() == null) {
71+
//throw new RuntimeException("SimpleAggregate is null");
72+
}
73+
instance.setSimpleAggregate(instance.getSimpleAggregate() == null ? new SimpleAggregate() : null);
74+
75+
qTracker.reset();
76+
77+
instance.setSimpleAggregate(null);
78+
uow.registerObject(instance);
79+
uow.commit();
80+
81+
if (qTracker.getQueries().size() > 0) {
82+
throw new RuntimeException("Unexpected query was executed: " + qTracker.getQueries());
83+
}
84+
}
85+
86+
}
87+
}

‎foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/models/aggregate/SimpleEntity.java

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Copyright (c) 1998, 2018 Oracle and/or its affiliates. All rights reserved.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0,
7+
* or the Eclipse Distribution License v. 1.0 which is available at
8+
* http://www.eclipse.org/org/documents/edl-v10.php.
9+
*
10+
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
11+
*/
12+
package org.eclipse.persistence.testing.models.aggregate;
13+
14+
import org.eclipse.persistence.tools.schemaframework.TableDefinition;
15+
16+
public class SimpleEntity {
17+
private String id;
18+
private String field;
19+
private SimpleAggregate simpleAggregate;
20+
21+
public String getId() {
22+
return id;
23+
}
24+
25+
public void setId(String id) {
26+
this.id = id;
27+
}
28+
29+
public String getField() {
30+
return field;
31+
}
32+
33+
public void setField(String field) {
34+
this.field = field;
35+
}
36+
37+
public SimpleAggregate getSimpleAggregate() {
38+
return simpleAggregate;
39+
}
40+
41+
public void setSimpleAggregate(SimpleAggregate simpleAggregate) {
42+
this.simpleAggregate = simpleAggregate;
43+
}
44+
45+
public static TableDefinition tableDefinition() {
46+
TableDefinition definition = new TableDefinition();
47+
48+
definition.setName("SIMPLEENTITY");
49+
50+
definition.addIdentityField("ID", String.class, 15);
51+
definition.addField("FIELD", String.class, 15);
52+
definition.addField("CONTENT", String.class, 20);
53+
54+
return definition;
55+
}
56+
}

‎foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/aggregate/SimpleAggregateTestModel.java

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright (c) 1998, 2018 Oracle and/or its affiliates. All rights reserved.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0,
7+
* or the Eclipse Distribution License v. 1.0 which is available at
8+
* http://www.eclipse.org/org/documents/edl-v10.php.
9+
*
10+
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
11+
*/
12+
package org.eclipse.persistence.testing.tests.aggregate;
13+
14+
import org.eclipse.persistence.testing.framework.TestModel;
15+
import org.eclipse.persistence.testing.framework.TestSuite;
16+
import org.eclipse.persistence.testing.models.aggregate.SimpleAggregateSystem;
17+
18+
public class SimpleAggregateTestModel extends TestModel {
19+
public SimpleAggregateTestModel() {
20+
setDescription("This model tests reading/writing/deleting of the simple aggregate model.");
21+
}
22+
23+
@Override
24+
public void addForcedRequiredSystems() {
25+
//We need to ensure that the correct database schema is created
26+
addForcedRequiredSystem(new SimpleAggregateSystem());
27+
}
28+
29+
@Override
30+
public void addRequiredSystems() {
31+
}
32+
33+
@Override
34+
public void addTests() {
35+
addTest(getUpdateObjectTestSuite());
36+
}
37+
38+
public static TestSuite getUpdateObjectTestSuite() {
39+
TestSuite suite = new TestSuite();
40+
suite.setName("AggregateUpdateObjectTestSuite");
41+
suite.setDescription("This suite tests the updating of each object in the aggregate model.");
42+
43+
return suite;
44+
}
45+
46+
public static junit.framework.TestSuite suite() {
47+
return new SimpleAggregateTestModel();
48+
}
49+
}

‎foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/descriptors/ObjectBuilder.java

+18
Original file line numberDiff line numberDiff line change
@@ -4639,4 +4639,22 @@ public boolean hasCacheIndexesInSopObject() {
46394639
public AbstractRecord createRecordFromXMLContext(XMLContext context) {
46404640
return createRecord((AbstractSession)context.getSession());
46414641
}
4642+
4643+
public boolean checkNull(Object o, AbstractSession session) {
4644+
if (o == null) {
4645+
return true;
4646+
}
4647+
// PERF: Avoid iterator.
4648+
final List mappings = this.descriptor.getMappings();
4649+
for (int index = 0; index < mappings.size(); index++) {
4650+
final DatabaseMapping mapping = (DatabaseMapping)mappings.get(index);
4651+
4652+
final Object value = mapping.getAttributeValueFromObject(o);
4653+
if (value != null) {
4654+
return false;
4655+
}
4656+
}
4657+
4658+
return true;
4659+
}
46424660
}

‎foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/mappings/AggregateMapping.java

+12-1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.eclipse.persistence.exceptions.*;
3737
import org.eclipse.persistence.expressions.*;
3838
import org.eclipse.persistence.internal.descriptors.*;
39+
import org.eclipse.persistence.internal.helper.DatabaseField;
3940
import org.eclipse.persistence.internal.helper.IdentityHashSet;
4041
import org.eclipse.persistence.internal.identitymaps.CacheKey;
4142
import org.eclipse.persistence.internal.security.PrivilegedAccessHelper;
@@ -316,7 +317,9 @@ public ChangeRecord compareForChange(Object clone, Object backup, ObjectChangeSe
316317

317318
if (!owner.isNew()) {
318319
backupAttribute = getAttributeValueFromObject(backup);
319-
if ((cloneAttribute == null) && (backupAttribute == null)) {
320+
if ((cloneAttribute == null || backupAttribute == null) &&
321+
checkNull(cloneAttribute, session) && checkNull(backupAttribute, session)
322+
) {
320323
return null;// no change
321324
}
322325
if ((cloneAttribute != null) && (backupAttribute != null) && (!cloneAttribute.getClass().equals(backupAttribute.getClass()))) {
@@ -354,6 +357,14 @@ public ChangeRecord compareForChange(Object clone, Object backup, ObjectChangeSe
354357
return changeRecord;
355358
}
356359

360+
public boolean checkNull(final Object o, final AbstractSession session) {
361+
if (o == null) {
362+
return true;
363+
}
364+
final ObjectBuilder builder = getObjectBuilder(o, session);
365+
return builder.checkNull(o, session);
366+
}
367+
357368
/**
358369
* INTERNAL:
359370
* Compare the attributes belonging to this mapping for the objects.

0 commit comments

Comments
 (0)