Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[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 #536

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}

}
Original file line number Diff line number Diff line change
@@ -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());
}
}

}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()))) {
Expand Down Expand Up @@ -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.
Expand Down