Skip to content

Commit fc72bd2

Browse files
eperezeperez
committed
Bug fix: When saving an entity, that was saved with an aggregate object that contains null values, with a null value for an aggregate then EclipseLink will still generate an update statement for that record field changing it from NULL to NULL, but it should not do that as there was no change.
1 parent 2637c10 commit fc72bd2

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)