From c6e5bb399980f2922ec42c18e5d4aa0ae055b68e Mon Sep 17 00:00:00 2001 From: Will Dazey Date: Tue, 28 Feb 2023 15:40:50 -0600 Subject: [PATCH 1/2] Issue 1815: Transforming object types for collections Signed-off-by: Will Dazey --- .../expressions/CollectionExpression.java | 26 ++- .../expressions/ParameterExpression.java | 19 ++ .../expressions/QueryKeyExpression.java | 204 +++++++++++++++--- 3 files changed, 214 insertions(+), 35 deletions(-) diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/CollectionExpression.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/CollectionExpression.java index e3317bb9b74..0f2499c290b 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/CollectionExpression.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/CollectionExpression.java @@ -38,13 +38,33 @@ public CollectionExpression(Object newValue, Expression baseExpression) { @Override public void printSQL(ExpressionSQLPrinter printer) { - Object value = this.value; + Object objectValue = this.value; if(this.localBase != null) { - value = this.localBase.getFieldValue(value, getSession()); + if(objectValue instanceof Collection) { + Collection values = (Collection)objectValue; + Vector fieldValues = new Vector<>(values.size()); + for (Object value : values) { + Object translated = value; + if (!(value instanceof Expression)){ + translated = this.localBase.getFieldValue(value, getSession()); + } + fieldValues.add(translated); + } + objectValue = fieldValues; + } } - printer.printList((Collection)value, this.canBind); + printer.printList((Collection)objectValue, this.canBind); } +// @Override +// public void printSQL(ExpressionSQLPrinter printer) { +// Object value = this.value; +// if(this.localBase != null) { +// value = this.localBase.getFieldValue(value, getSession()); +// } +// printer.printList((Collection)value, this.canBind); +// } + /** * INTERNAL: * Return the value for in memory comparison. diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/ParameterExpression.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/ParameterExpression.java index 320688708da..e653cc87e78 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/ParameterExpression.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/ParameterExpression.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Vector; import org.eclipse.persistence.descriptors.ClassDescriptor; import org.eclipse.persistence.exceptions.QueryException; @@ -320,6 +321,24 @@ public Object getValue(AbstractRecord translationRow, DatabaseQuery query, Abstr value = this.localBase.getFieldValue(value, session); } +// // Convert the value to the correct type, i.e. object type mappings. +// if (this.localBase != null) { +// if(value instanceof Collection) { +// Collection values = (Collection)value; +// Vector fieldValues = new Vector<>(values.size()); +// for (Object v : values) { +// Object translated = v; +// if (!(v instanceof Expression)){ +// translated = this.localBase.getFieldValue(v, getSession()); +// } +// fieldValues.add(translated); +// } +// value = fieldValues; +// } else { +// value = this.localBase.getFieldValue(value, session); +// } +// } + return value; } diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/QueryKeyExpression.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/QueryKeyExpression.java index 22cd3a12711..1b0b7bab86e 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/QueryKeyExpression.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/expressions/QueryKeyExpression.java @@ -1,5 +1,6 @@ /* - * Copyright (c) 1998, 2022 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023 IBM Corporation. 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 @@ -414,6 +415,57 @@ public List getSelectionFields(ReadQuery query) { } } +// /** +// * INTERNAL: +// * Transform the object-level value into a database-level value +// */ +// @Override +// public Object getFieldValue(Object objectValue, AbstractSession session) { +// DatabaseMapping mapping = getMapping(); +// Object fieldValue = objectValue; +// if (mapping != null) { +// if (mapping.isAbstractDirectMapping() || mapping.isDirectCollectionMapping()) { +// // CR#3623207, check for IN Collection here not in mapping. +// if (objectValue instanceof Collection) { +// // This can actually be a collection for IN within expressions... however it would be better for expressions to handle this. +// Collection values = (Collection)objectValue; +// Vector fieldValues = new Vector(values.size()); +// for (Iterator iterator = values.iterator(); iterator.hasNext();) { +// Object value = iterator.next(); +// if (!(value instanceof Expression)){ +// value = getFieldValue(value, session); +// } +// fieldValues.add(value); +// } +// fieldValue = fieldValues; +// } else { +// if (mapping.isAbstractColumnMapping()) { +// fieldValue = ((AbstractColumnMapping)mapping).getFieldValue(objectValue, session); +// } else if (mapping.isDirectCollectionMapping()) { +// fieldValue = ((DirectCollectionMapping)mapping).getFieldValue(objectValue, session); +// } +// } +// } else if ((objectValue instanceof Collection) && (mapping.isForeignReferenceMapping())) { +// // Was an IN with a collection of objects, extract their ids. +// List ids = new ArrayList(); +// for (Object object : ((Collection)objectValue)) { +// if ((mapping.getReferenceDescriptor() != null) && (mapping.getReferenceDescriptor().getJavaClass().isInstance(object))) { +// Object id = mapping.getReferenceDescriptor().getObjectBuilder().extractPrimaryKeyFromObject(object, session); +// if (id instanceof CacheId) { +// id = Arrays.asList(((CacheId)id).getPrimaryKey()); +// } +// ids.add(id); +// } else { +// ids.add(object); +// } +// } +// fieldValue = ids; +// } +// } +// +// return fieldValue; +// } + /** * INTERNAL: * Transform the object-level value into a database-level value @@ -424,47 +476,135 @@ public Object getFieldValue(Object objectValue, AbstractSession session) { Object fieldValue = objectValue; if (mapping != null) { if (mapping.isAbstractDirectMapping() || mapping.isDirectCollectionMapping()) { - // CR#3623207, check for IN Collection here not in mapping. - if (objectValue instanceof Collection) { - // This can actually be a collection for IN within expressions... however it would be better for expressions to handle this. - Collection values = (Collection)objectValue; - Vector fieldValues = new Vector(values.size()); - for (Iterator iterator = values.iterator(); iterator.hasNext();) { - Object value = iterator.next(); - if (!(value instanceof Expression)){ - value = getFieldValue(value, session); - } - fieldValues.add(value); - } - fieldValue = fieldValues; - } else { - if (mapping.isAbstractColumnMapping()) { - fieldValue = ((AbstractColumnMapping)mapping).getFieldValue(objectValue, session); - } else if (mapping.isDirectCollectionMapping()) { - fieldValue = ((DirectCollectionMapping)mapping).getFieldValue(objectValue, session); - } + if (mapping.isAbstractColumnMapping()) { + fieldValue = ((AbstractColumnMapping)mapping).getFieldValue(objectValue, session); + } else if (mapping.isDirectCollectionMapping()) { + fieldValue = ((DirectCollectionMapping)mapping).getFieldValue(objectValue, session); } } else if ((objectValue instanceof Collection) && (mapping.isForeignReferenceMapping())) { - // Was an IN with a collection of objects, extract their ids. - List ids = new ArrayList(); - for (Object object : ((Collection)objectValue)) { - if ((mapping.getReferenceDescriptor() != null) && (mapping.getReferenceDescriptor().getJavaClass().isInstance(object))) { - Object id = mapping.getReferenceDescriptor().getObjectBuilder().extractPrimaryKeyFromObject(object, session); - if (id instanceof CacheId) { - id = Arrays.asList(((CacheId)id).getPrimaryKey()); - } - ids.add(id); - } else { - ids.add(object); + if ((mapping.getReferenceDescriptor() != null) && (mapping.getReferenceDescriptor().getJavaClass().isInstance(objectValue))) { + Object id = mapping.getReferenceDescriptor().getObjectBuilder().extractPrimaryKeyFromObject(objectValue, session); + if (id instanceof CacheId) { + id = Arrays.asList(((CacheId)id).getPrimaryKey()); } + fieldValue = id; } - fieldValue = ids; } } return fieldValue; } +// /** +// * INTERNAL: +// * Transform the object-level value into a database-level value +// */ +// @Override +// public Object getFieldValue(Object objectValue, AbstractSession session) { +// DatabaseMapping mapping = getMapping(); +// Object fieldValue = objectValue; +// if (mapping != null) { +// if (mapping.isAbstractDirectMapping() || mapping.isDirectCollectionMapping()) { +// +//// if (objectValue instanceof Collection) { +//// // This can actually be a collection for IN within expressions... however it would be better for expressions to handle this. +//// Collection values = (Collection)objectValue; +//// +//// if(values.size() > 0 && ) { +//// +//// } +//// +//// Vector fieldValues = new Vector<>(values.size()); +//// Class cls = mapping.getAttributeClassification(); +//// +//// for (Object value : values) { +//// // If the element isn't the correct type +//// if(cls != null && !cls.isAssignableFrom(value.getClass())) { +//// value = getFieldValue(value, session); +//// } else if (!(value instanceof Expression)){ +//// value = getFieldValue(value, session); +//// } +//// fieldValues.add(value); +//// } +//// fieldValue = fieldValues; +//// } else { +//// if (mapping.isAbstractColumnMapping()) { +//// fieldValue = ((AbstractColumnMapping)mapping).getFieldValue(objectValue, session); +//// } else if (mapping.isDirectCollectionMapping()) { +//// fieldValue = ((DirectCollectionMapping)mapping).getFieldValue(objectValue, session); +//// } +//// } +// +// Class cls = mapping.getAttributeClassification(); +// +// // CR#3623207, check for IN Collection here not in mapping. +// // Translate the individual elements of a collection to match the mapping type. This is important for IN collections +// // However, only translate the elements if the mapping is not to a collection itself. Otherwise, we want to maintain a Collection for serialization +// if(objectValue instanceof Collection && cls != null && !cls.isAssignableFrom(objectValue.getClass())) { +// // This can actually be a collection for IN within expressions... however it would be better for expressions to handle this. +// Collection values = (Collection)objectValue; +// Vector fieldValues = new Vector<>(values.size()); +// for (Iterator iterator = values.iterator(); iterator.hasNext();) { +// Object value = iterator.next(); +// if (!(value instanceof Expression)) { +// if (mapping.isAbstractColumnMapping()) { +// value = ((AbstractColumnMapping)mapping).getFieldValue(value, session); +// } else if (mapping.isDirectCollectionMapping()) { +// value = ((DirectCollectionMapping)mapping).getFieldValue(value, session); +// } +// } +// fieldValues.add(value); +// } +// fieldValue = fieldValues; +// } else { +// if (mapping.isAbstractColumnMapping()) { +// fieldValue = ((AbstractColumnMapping)mapping).getFieldValue(objectValue, session); +// } else if (mapping.isDirectCollectionMapping()) { +// fieldValue = ((DirectCollectionMapping)mapping).getFieldValue(objectValue, session); +// } +// } +// +//// if (objectValue instanceof Collection +//// && (cls != null && !(cls.isArray() || cls.isInstance(Collection.class)))) { +//// // This can actually be a collection for IN within expressions... however it would be better for expressions to handle this. +//// Collection values = (Collection)objectValue; +//// Vector fieldValues = new Vector<>(values.size()); +//// for (Iterator iterator = values.iterator(); iterator.hasNext();) { +//// Object value = iterator.next(); +//// if (!(value instanceof Expression)){ +//// value = getFieldValue(value, session); +//// } +//// fieldValues.add(value); +//// } +//// fieldValue = fieldValues; +//// } else { +//// if (mapping.isAbstractColumnMapping()) { +//// fieldValue = ((AbstractColumnMapping)mapping).getFieldValue(objectValue, session); +//// } else if (mapping.isDirectCollectionMapping()) { +//// fieldValue = ((DirectCollectionMapping)mapping).getFieldValue(objectValue, session); +//// } +//// } +// } else if ((objectValue instanceof Collection) && (mapping.isForeignReferenceMapping())) { +// // Was an IN with a collection of objects, extract their ids. +// List ids = new ArrayList<>(); +// for (Object object : ((Collection)objectValue)) { +// if ((mapping.getReferenceDescriptor() != null) && (mapping.getReferenceDescriptor().getJavaClass().isInstance(object))) { +// Object id = mapping.getReferenceDescriptor().getObjectBuilder().extractPrimaryKeyFromObject(object, session); +// if (id instanceof CacheId) { +// id = Arrays.asList(((CacheId)id).getPrimaryKey()); +// } +// ids.add(id); +// } else { +// ids.add(object); +// } +// } +// fieldValue = ids; +// } +// } +// +// return fieldValue; +// } + @Override public DatabaseMapping getMapping() { if (!hasMapping) { From d8fb0446e779ed4fcbe43bb077d10f1690df3c84 Mon Sep 17 00:00:00 2001 From: Will Dazey Date: Mon, 6 Mar 2023 11:53:08 -0600 Subject: [PATCH 2/2] Issue 1815: Add testing Signed-off-by: Will Dazey --- .../test/collection/TestBasicCollection.java | 189 ++++++++++++++++++ .../jpa/test/collection/model/CityEntity.java | 51 +++++ 2 files changed, 240 insertions(+) create mode 100644 jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/collection/TestBasicCollection.java create mode 100644 jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/collection/model/CityEntity.java diff --git a/jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/collection/TestBasicCollection.java b/jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/collection/TestBasicCollection.java new file mode 100644 index 00000000000..9c3c261e008 --- /dev/null +++ b/jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/collection/TestBasicCollection.java @@ -0,0 +1,189 @@ +/* + * Copyright (c) 2023 IBM Corporation. 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 + */ + +// Contributors: +package org.eclipse.persistence.jpa.test.collection; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityManagerFactory; +import jakarta.persistence.Query; +import jakarta.persistence.TypedQuery; + +import java.util.List; +import java.util.Set; + +import org.eclipse.persistence.jpa.test.framework.DDLGen; +import org.eclipse.persistence.jpa.test.framework.Emf; +import org.eclipse.persistence.jpa.test.framework.EmfRunner; +import org.eclipse.persistence.jpa.test.framework.Property; +import org.eclipse.persistence.jpa.test.collection.model.CityEntity; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(EmfRunner.class) +public class TestBasicCollection { + + @Emf(createTables = DDLGen.DROP_CREATE, + classes = { CityEntity.class }, + properties = { @Property(name = "eclipselink.logging.level", value = "ALL"), + @Property(name = "eclipselink.logging.level.sql", value = "FINE"), + @Property(name = "eclipselink.logging.parameters", value = "true")}) + private EntityManagerFactory emf; + + @Test + public void testUpdateBasicCollectionWithQuery() { + Set origSet = Set.of(608); + + EntityManager em = emf.createEntityManager(); + try { + em.getTransaction().begin(); + em.persist(new CityEntity("Minneapolis", origSet)); + em.getTransaction().commit(); + } finally { + if (em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + if(em.isOpen()) { + em.clear(); + em.close(); + } + } + + em = emf.createEntityManager(); + try { + em.getTransaction().begin(); + CityEntity find1 = em.find(CityEntity.class, "Minneapolis"); + + Assert.assertNotNull(find1); + Assert.assertNotNull(find1.getAreaCodes()); + Assert.assertEquals(origSet.size(), find1.getAreaCodes().size()); + Assert.assertTrue(origSet.containsAll(find1.getAreaCodes())); + em.getTransaction().commit(); + } finally { + if (em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + if(em.isOpen()) { + em.clear(); + em.close(); + } + } + + Set updatedSet = Set.of(563, 456); + + em = emf.createEntityManager(); + try { + em.getTransaction().begin(); + Query q = em.createQuery("UPDATE CityEntity o SET o.areaCodes=?1 WHERE o.name=?2"); + q.setParameter(1, updatedSet); + q.setParameter(2, "Minneapolis"); + q.executeUpdate(); + em.getTransaction().commit(); + } finally { + if (em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + if(em.isOpen()) { + em.clear(); + em.close(); + } + } + + em = emf.createEntityManager(); + try { + em.getTransaction().begin(); + CityEntity find2 = em.find(CityEntity.class, "Minneapolis"); + + Assert.assertNotNull(find2); + Assert.assertNotNull(find2.getAreaCodes()); + Assert.assertEquals(updatedSet.size(), find2.getAreaCodes().size()); + Assert.assertTrue(updatedSet.containsAll(find2.getAreaCodes())); + em.getTransaction().commit(); + } finally { + if (em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + if(em.isOpen()) { + em.clear(); + em.close(); + } + } + } + + @Test + public void testINBasicCollectionWithQuery() { + Set origSet = Set.of(234, 789); + + EntityManager em = emf.createEntityManager(); + try { + em.getTransaction().begin(); + em.persist(new CityEntity("Los Angeles", origSet)); + em.getTransaction().commit(); + } finally { + if (em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + if(em.isOpen()) { + em.clear(); + em.close(); + } + } + + em = emf.createEntityManager(); + try { + em.getTransaction().begin(); + CityEntity find1 = em.find(CityEntity.class, "Los Angeles"); + + Assert.assertNotNull(find1); + Assert.assertNotNull(find1.getAreaCodes()); + Assert.assertEquals(origSet.size(), find1.getAreaCodes().size()); + Assert.assertTrue("Expected " + origSet + " / actual " + find1.getAreaCodes(), origSet.containsAll(find1.getAreaCodes())); + em.getTransaction().commit(); + } finally { + if (em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + if(em.isOpen()) { + em.clear(); + em.close(); + } + } + + Set set1 = Set.of(123, 234); + Set set2 = Set.of(345); + Set set3 = Set.of(234, 789); + Set set4 = Set.of(678); + Set> searchSet = Set.of(set1, set2, set3, set4); + + em = emf.createEntityManager(); + try { + em.getTransaction().begin(); + TypedQuery q = em.createQuery("SELECT e FROM CityEntity e WHERE e.areaCodes IN ?1", CityEntity.class); + q.setParameter(1, searchSet); + List res = q.getResultList(); + + Assert.assertEquals(1, res.size()); + CityEntity find2 = res.get(1); + Assert.assertTrue(origSet.containsAll(find2.getAreaCodes())); + em.getTransaction().commit(); + } finally { + if (em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + if(em.isOpen()) { + em.clear(); + em.close(); + } + } + } +} diff --git a/jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/collection/model/CityEntity.java b/jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/collection/model/CityEntity.java new file mode 100644 index 00000000000..0430698ebcc --- /dev/null +++ b/jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/collection/model/CityEntity.java @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2023 IBM Corporation. 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 + */ + +// Contributors: +package org.eclipse.persistence.jpa.test.collection.model; + +import java.util.Set; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; + +@Entity +public class CityEntity { + + @Id + public String name; + + public Set areaCodes; + + public CityEntity() {} + + public CityEntity(String name, Set areaCodes) { + this.name = name; + this.areaCodes = areaCodes; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Set getAreaCodes() { + return areaCodes; + } + + public void setAreaCodes(Set areaCodes) { + this.areaCodes = areaCodes; + } +}