Fix a few rough edges in updatable entity views and finalize Hibernat…

…e workaround
Blazebit · Nov 8, 2018 · 0a8a00d · 0a8a00d
1 parent 531f920
commit 0a8a00d
Show file tree

Hide file tree

Showing 18 changed files with 55 additions and 61 deletions.
diff --git a/core/api/src/main/java/com/blazebit/persistence/spi/JpaProvider.java b/core/api/src/main/java/com/blazebit/persistence/spi/JpaProvider.java
@@ -496,7 +496,7 @@ public interface JpaProvider {
      * @return true if needed, else false
      * @since 1.3.0
      */
-    public boolean needsElementCollectionIdCutoffForCompositeIdOwner();
+    public boolean needsElementCollectionIdCutoff();
 
     /**
      * Enables query result caching for the given query.

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java b/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java
@@ -1968,7 +1968,7 @@ protected List<EntityFunctionNode> getEntityFunctionNodes(Query baseQuery) {
                 syntheticPredicate = syntheticPredicate.replace(exampleQuerySqlAlias, valuesTableSqlAlias);
             }
 
-            entityFunctionNodes.add(new EntityFunctionNode(valuesClause, valuesAliases, clazz, valuesTableSqlAlias, syntheticPredicate));
+            entityFunctionNodes.add(new EntityFunctionNode(valuesClause, valuesAliases, node.getInternalEntityType().getName(), valuesTableSqlAlias, syntheticPredicate));
         }
         return entityFunctionNodes;
     }

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/CachingJpaProvider.java b/core/impl/src/main/java/com/blazebit/persistence/impl/CachingJpaProvider.java
@@ -329,8 +329,8 @@ public boolean supportsSingleValuedAssociationNaturalIdExpressions() {
     }
 
     @Override
-    public boolean needsElementCollectionIdCutoffForCompositeIdOwner() {
-        return jpaProvider.needsElementCollectionIdCutoffForCompositeIdOwner();
+    public boolean needsElementCollectionIdCutoff() {
+        return jpaProvider.needsElementCollectionIdCutoff();
     }
 
     @Override

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/JoinNode.java b/core/impl/src/main/java/com/blazebit/persistence/impl/JoinNode.java
@@ -37,7 +37,6 @@
 import com.blazebit.persistence.parser.util.JpaMetamodelUtils;
 
 import javax.persistence.metamodel.Attribute;
-import javax.persistence.metamodel.EmbeddableType;
 import javax.persistence.metamodel.EntityType;
 import javax.persistence.metamodel.ManagedType;
 import javax.persistence.metamodel.SingularAttribute;
@@ -804,8 +803,7 @@ public void appendDeReference(StringBuilder sb, String property, boolean renderT
         if (property != null && valuesTypeName == null) {
             Set<SingularAttribute<?, ?>> idAttributes;
             if (requiresElementCollectionIdCutoff && parentTreeNode != null && parentTreeNode.getAttribute().getPersistentAttributeType() == Attribute.PersistentAttributeType.ELEMENT_COLLECTION
-                    // Only relevant when the owning entity type has an embedded id
-                    && property.endsWith(".id") && ((idAttributes = JpaMetamodelUtils.getIdAttributes(parent.getEntityType())).size() > 1 || idAttributes.iterator().next().getType() instanceof EmbeddableType<?>)) {
+                    && property.endsWith(".id")) {
                 // See https://hibernate.atlassian.net/browse/HHH-13045 for details
                 sb.append('.').append(property, 0, property.length() - ".id".length());
             } else {

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/JpaUtils.java b/core/impl/src/main/java/com/blazebit/persistence/impl/JpaUtils.java
@@ -23,19 +23,16 @@
 import com.blazebit.persistence.parser.expression.ParameterExpression;
 import com.blazebit.persistence.parser.expression.PathExpression;
 import com.blazebit.persistence.parser.expression.PropertyExpression;
-import com.blazebit.persistence.parser.util.JpaMetamodelUtils;
 import com.blazebit.persistence.spi.ExtendedAttribute;
 import com.blazebit.persistence.spi.ExtendedManagedType;
 import com.blazebit.persistence.spi.JoinTable;
 import com.blazebit.persistence.spi.JpaMetamodelAccessor;
 import com.blazebit.persistence.spi.JpaProvider;
 
 import javax.persistence.metamodel.Attribute;
-import javax.persistence.metamodel.EmbeddableType;
 import javax.persistence.metamodel.EntityType;
 import javax.persistence.metamodel.ManagedType;
 import javax.persistence.metamodel.PluralAttribute;
-import javax.persistence.metamodel.SingularAttribute;
 import javax.persistence.metamodel.Type;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
@@ -48,7 +45,6 @@
 import java.util.Map;
 import java.util.NavigableSet;
 import java.util.Queue;
-import java.util.Set;
 import java.util.TreeSet;
 
 /**
@@ -69,8 +65,7 @@ public static void expandBindings(EntityType<?> bindType, Map<String, Integer> b
         EntityMetamodel metamodel = queryBuilder.mainQuery.metamodel;
         JpaMetamodelAccessor jpaMetamodelAccessor = jpaProvider.getJpaMetamodelAccessor();
 
-        Set<SingularAttribute<?, ?>> idAttributes;
-        boolean needsElementCollectionIdCutoffForCompositeIdOwner = jpaProvider.needsElementCollectionIdCutoffForCompositeIdOwner() && ((idAttributes = JpaMetamodelUtils.getIdAttributes(bindType)).size() > 1 || idAttributes.iterator().next().getType() instanceof EmbeddableType<?>);
+        boolean needsElementCollectionIdCutoffForCompositeIdOwner = jpaProvider.needsElementCollectionIdCutoff();
         final Queue<String> attributeQueue = new ArrayDeque<>(bindingMap.keySet());
         while (!attributeQueue.isEmpty()) {
             final String attributeName = attributeQueue.remove();
@@ -160,7 +155,7 @@ public static void expandBindings(EntityType<?> bindType, Map<String, Integer> b
                             }
                         }
                     } else if (selectExpression instanceof ParameterExpression) {
-                        final Collection<String> embeddedPropertyNames = getEmbeddedPropertyPaths(attributeEntries, attributeName, jpaProvider.needsElementCollectionIdCutoffForCompositeIdOwner());
+                        final Collection<String> embeddedPropertyNames = getEmbeddedPropertyPaths(attributeEntries, attributeName, jpaProvider.needsElementCollectionIdCutoff());
 
                         if (embeddedPropertyNames.size() > 0) {
                             ParameterExpression parameterExpression = (ParameterExpression) selectExpression;

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/ResolvingQueryGenerator.java b/core/impl/src/main/java/com/blazebit/persistence/impl/ResolvingQueryGenerator.java
@@ -280,7 +280,7 @@ public String getQueryString() {
 
                 // TODO: this is a hibernate specific integration detail
                 // Replace the subview subselect that is generated for this subselect
-                String entityName = node.getEntityClass().getSimpleName();
+                String entityName = node.getEntityName();
                 arguments.add(new StringLiteral(entityName));
                 arguments.add(new StringLiteral(valuesClause));
                 arguments.add(new StringLiteral(valuesAliases == null ? "" : valuesAliases));
@@ -478,7 +478,7 @@ public void visit(PathExpression expression) {
                     sb.append(aliasPrefix);
                 }
 
-                baseNode.appendDeReference(sb, field, renderTreat, externalRepresentation, jpaProvider.needsElementCollectionIdCutoffForCompositeIdOwner());
+                baseNode.appendDeReference(sb, field, renderTreat, externalRepresentation, jpaProvider.needsElementCollectionIdCutoff());
             }
 
             if (addTypeCaseWhen) {

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/query/CustomQuerySpecification.java b/core/impl/src/main/java/com/blazebit/persistence/impl/query/CustomQuerySpecification.java
@@ -391,7 +391,7 @@ protected StringBuilder applySqlTransformations(String sqlQuery) {
 
             // TODO: this is a hibernate specific integration detail
             // Replace the subview subselect that is generated for this subselect
-            String entityName = node.getEntityClass().getSimpleName();
+            String entityName = node.getEntityName();
             final String subselect = "( select * from " + entityName + " )";
             final String subselectTableExpr = subselect + " " + valuesTableSqlAlias;
             int subselectIndex = sb.indexOf(subselectTableExpr, 0);

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/query/EntityFunctionNode.java b/core/impl/src/main/java/com/blazebit/persistence/impl/query/EntityFunctionNode.java
@@ -25,14 +25,14 @@ public class EntityFunctionNode {
 
     private final String valuesClause;
     private final String valuesAliases;
-    private final Class<?> entityClass;
+    private final String entityName;
     private final String tableAlias;
     private final String syntheticPredicate;
 
-    public EntityFunctionNode(String valuesClause, String valuesAliases, Class<?> entityClass, String tableAlias, String syntheticPredicate) {
+    public EntityFunctionNode(String valuesClause, String valuesAliases, String entityName, String tableAlias, String syntheticPredicate) {
         this.valuesClause = valuesClause;
         this.valuesAliases = valuesAliases;
-        this.entityClass = entityClass;
+        this.entityName = entityName;
         this.tableAlias = tableAlias;
         this.syntheticPredicate = syntheticPredicate;
     }
@@ -45,8 +45,8 @@ public String getValuesAliases() {
         return valuesAliases;
     }
 
-    public Class<?> getEntityClass() {
-        return entityClass;
+    public String getEntityName() {
+        return entityName;
     }
 
     public String getTableAlias() {

diff --git a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/GroupByTest.java b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/GroupByTest.java
@@ -18,22 +18,16 @@
 
 import static org.junit.Assert.assertEquals;
 
-import com.blazebit.persistence.Criteria;
-import com.blazebit.persistence.spi.CriteriaBuilderConfiguration;
-import com.blazebit.persistence.spi.DbmsDialect;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import com.blazebit.persistence.CriteriaBuilder;
-import com.blazebit.persistence.testsuite.base.jpa.category.NoDB2;
 import com.blazebit.persistence.testsuite.base.jpa.category.NoDatanucleus;
 import com.blazebit.persistence.testsuite.base.jpa.category.NoFirebird;
 import com.blazebit.persistence.testsuite.base.jpa.category.NoH2;
 import com.blazebit.persistence.testsuite.base.jpa.category.NoMySQL;
-import com.blazebit.persistence.testsuite.base.jpa.category.NoOracle;
 import com.blazebit.persistence.testsuite.base.jpa.category.NoPostgreSQL;
 import com.blazebit.persistence.testsuite.base.jpa.category.NoSQLite;
-import com.blazebit.persistence.testsuite.base.jpa.category.NoMSSQL;
 import com.blazebit.persistence.testsuite.entity.Document;
 
 /**
@@ -122,7 +116,11 @@ public void testGroupByEmbeddable() {
                 .groupBy("names");
 
         if (jpaProvider.supportsSingleValuedAssociationIdExpressions()) {
-            assertEquals("SELECT d.id FROM Document d LEFT JOIN d.names names_1 GROUP BY names_1.intIdEntity.id, names_1.primaryName, names_1.secondaryName, d.id", cb.getQueryString());
+            if (jpaProvider.needsElementCollectionIdCutoff()) {
+                assertEquals("SELECT d.id FROM Document d LEFT JOIN d.names names_1 GROUP BY names_1.intIdEntity, names_1.primaryName, names_1.secondaryName, d.id", cb.getQueryString());
+            } else {
+                assertEquals("SELECT d.id FROM Document d LEFT JOIN d.names names_1 GROUP BY names_1.intIdEntity.id, names_1.primaryName, names_1.secondaryName, d.id", cb.getQueryString());
+            }
         } else {
             assertEquals("SELECT d.id FROM Document d LEFT JOIN d.names names_1 LEFT JOIN names_1.intIdEntity intIdEntity_1 GROUP BY intIdEntity_1.id, names_1.primaryName, names_1.secondaryName, d.id", cb.getQueryString());
         }
@@ -148,7 +146,11 @@ public void testGroupByElementCollectionValue() {
                 .groupBy("VALUE(nameMap)");
 
         if (jpaProvider.supportsSingleValuedAssociationIdExpressions()) {
-            assertEquals("SELECT d.id FROM Document d LEFT JOIN d.nameMap nameMap_1 GROUP BY nameMap_1.intIdEntity.id, nameMap_1.primaryName, nameMap_1.secondaryName, d.id", cb.getQueryString());
+            if (jpaProvider.needsElementCollectionIdCutoff()) {
+                assertEquals("SELECT d.id FROM Document d LEFT JOIN d.nameMap nameMap_1 GROUP BY nameMap_1.intIdEntity, nameMap_1.primaryName, nameMap_1.secondaryName, d.id", cb.getQueryString());
+            } else {
+                assertEquals("SELECT d.id FROM Document d LEFT JOIN d.nameMap nameMap_1 GROUP BY nameMap_1.intIdEntity.id, nameMap_1.primaryName, nameMap_1.secondaryName, d.id", cb.getQueryString());
+            }
         } else {
             assertEquals("SELECT d.id FROM Document d LEFT JOIN d.nameMap nameMap_1 LEFT JOIN nameMap_1.intIdEntity intIdEntity_1 GROUP BY intIdEntity_1.id, nameMap_1.primaryName, nameMap_1.secondaryName, d.id", cb.getQueryString());
         }

diff --git a/.../main/java/com/blazebit/persistence/view/impl/entity/LoadOrPersistViewToEntityMapper.java b/.../main/java/com/blazebit/persistence/view/impl/entity/LoadOrPersistViewToEntityMapper.java
@@ -66,7 +66,7 @@ public Object flushToEntity(UpdateContext context, Object entity, Object view) {
                 if (persistAllowed && persistUpdater.containsKey(viewTypeClass) && !((EntityViewProxy) view).$$_isNew()) {
                     // If that create view object was previously persisted, we won't persist it again, nor update, but just load it
                 } else {
-                    throw new IllegalArgumentException("Couldn't load entity object for attribute '" + attributeLocation + "'. Expected subview of on of the types " + names(viewTypeClasses) + " but got: " + view);
+                    throw new IllegalArgumentException("Couldn't load entity object for attribute '" + attributeLocation + "'. Expected subview of one of the types " + names(viewTypeClasses) + " but got: " + view);
                 }
             }
         }

diff --git a/...ew/impl/src/main/java/com/blazebit/persistence/view/impl/metamodel/AbstractAttribute.java b/...ew/impl/src/main/java/com/blazebit/persistence/view/impl/metamodel/AbstractAttribute.java
@@ -783,7 +783,7 @@ public void checkAttribute(ManagedType<?> managedType, MetamodelBuildingContext
                         for (Type<?> updateCascadeAllowedSubtype : getUpdateCascadeAllowedSubtypes()) {
                             ManagedViewType<?> managedViewType = (ManagedViewType<?>) updateCascadeAllowedSubtype;
                             if (managedViewType.isUpdatable()) {
-                                context.addError("Invalid use of @UpdatableEntityView type '" + managedViewType.getJavaType().getName() + "' for the " + getLocation() + ". Consider using a read-only view type instead! " +
+                                context.addError("Invalid use of @UpdatableEntityView type '" + managedViewType.getJavaType().getName() + "' for the " + getLocation() + ". Consider using a read-only view type instead or use @AllowUpdatableEntityViews! " +
                                         "For further information on this topic, please consult the documentation https://persistence.blazebit.com/documentation/entity-view/manual/en_US/index.html#updatable-mappings-subview");
                             }
                         }

diff --git a/...pl/src/main/java/com/blazebit/persistence/view/impl/metamodel/MethodAttributeMapping.java b/...pl/src/main/java/com/blazebit/persistence/view/impl/metamodel/MethodAttributeMapping.java
@@ -501,7 +501,7 @@ private Map<ViewMapping, Boolean> initializeDependentSubtypeMappingsAuto(final M
         }
 
         final Map<ViewMapping, Boolean> subtypeMappings = new HashMap<>(subtypes.size());
-        for (Class<?> type : subtypes) {
+        for (final Class<?> type : subtypes) {
             final ViewMapping subtypeMapping = context.getViewMapping(type);
             if (subtypeMapping == null) {
                 unknownSubviewType(type);
@@ -515,9 +515,10 @@ public void run() {
                             @Override
                             public void run() {
                                 Set<Class<?>> dependencies = new HashSet<>();
-                                dependencies.add(getDeclaringView().getEntityViewClass());
+                                dependencies.add(MethodAttributeMapping.this.getDeclaringView().getEntityViewClass());
                                 dependencies.add(clazz);
                                 if (!subtypeMapping.validateDependencies(context, dependencies, MethodAttributeMapping.this, clazz, false)) {
+                                    // validateCascadeSubtypeMappings will remove this when this speculation was invalid
                                     subtypeMappings.put(subtypeMapping, Boolean.FALSE);
                                 }
                             }

diff --git a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/proxy/ProxyFactory.java b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/proxy/ProxyFactory.java
@@ -1462,30 +1462,9 @@ private CtMethod addSetter(AbstractMethodAttribute<?, ?> attribute, CtClass cc,
             sb.append("\t}\n");
         }
 
-        if (attribute != null && attribute.getDirtyStateIndex() != -1) {
-            // Unset previous object parent
-            if (attribute.isCollection()) {
-                sb.append("\tif ($0.").append(fieldName).append(" != null && $0.").append(fieldName).append(" != $1) {\n");
-                if (attribute instanceof MapAttribute<?, ?, ?>) {
-                    sb.append("\t\tif ($0.").append(fieldName).append(" instanceof ").append(RecordingMap.class.getName()).append(") {\n");
-                    sb.append("\t\t\t((").append(RecordingMap.class.getName()).append(") $0.").append(fieldName).append(").$$_unsetParent();\n");
-                    sb.append("\t\t}\n");
-                } else {
-                    sb.append("\t\tif ($0.").append(fieldName).append(" instanceof ").append(RecordingCollection.class.getName()).append(") {\n");
-                    sb.append("\t\t\t((").append(RecordingCollection.class.getName()).append(") $0.").append(fieldName).append(").$$_unsetParent();\n");
-                    sb.append("\t\t}\n");
-                }
-                sb.append("\t}\n");
-            } else if (attribute.isSubview()) {
-                sb.append("\tif ($0.").append(fieldName).append(" != $1 && $0.").append(fieldName).append(" instanceof ").append(BasicDirtyTracker.class.getName()).append(") {\n");
-                sb.append("\t\t((").append(BasicDirtyTracker.class.getName()).append(") $0.").append(fieldName).append(").$$_unsetParent();\n");
-                sb.append("\t}\n");
-            }
-        }
-
         if (attribute != null && attribute.isUpdatable()) {
             // Collections do type checking in their recording collection implementations
-            if (!attribute.isCollection() && (attribute.isPersistCascaded() || attribute.isUpdateCascaded())) {
+            if (!attribute.isCollection() && dirtyChecking) {
                 // Only consider subviews here for now
                 if (attribute.isSubview()) {
                     String subtypeArray = addAllowedSubtypeField(cc, attribute);
@@ -1509,6 +1488,25 @@ private CtMethod addSetter(AbstractMethodAttribute<?, ?> attribute, CtClass cc,
         }
 
         if (attribute != null && attribute.getDirtyStateIndex() != -1) {
+            // Unset previous object parent
+            if (attribute.isCollection()) {
+                sb.append("\tif ($0.").append(fieldName).append(" != null && $0.").append(fieldName).append(" != $1) {\n");
+                if (attribute instanceof MapAttribute<?, ?, ?>) {
+                    sb.append("\t\tif ($0.").append(fieldName).append(" instanceof ").append(RecordingMap.class.getName()).append(") {\n");
+                    sb.append("\t\t\t((").append(RecordingMap.class.getName()).append(") $0.").append(fieldName).append(").$$_unsetParent();\n");
+                    sb.append("\t\t}\n");
+                } else {
+                    sb.append("\t\tif ($0.").append(fieldName).append(" instanceof ").append(RecordingCollection.class.getName()).append(") {\n");
+                    sb.append("\t\t\t((").append(RecordingCollection.class.getName()).append(") $0.").append(fieldName).append(").$$_unsetParent();\n");
+                    sb.append("\t\t}\n");
+                }
+                sb.append("\t}\n");
+            } else if (attribute.isSubview()) {
+                sb.append("\tif ($0.").append(fieldName).append(" != $1 && $0.").append(fieldName).append(" instanceof ").append(BasicDirtyTracker.class.getName()).append(") {\n");
+                sb.append("\t\t((").append(BasicDirtyTracker.class.getName()).append(") $0.").append(fieldName).append(").$$_unsetParent();\n");
+                sb.append("\t}\n");
+            }
+
             int mutableStateIndex = attribute.getDirtyStateIndex();
             if (mutableStateField != null) {
                 // this.mutableState[mutableStateIndex] = $1

diff --git a/.../main/java/com/blazebit/persistence/integration/datanucleus/DataNucleus51JpaProvider.java b/.../main/java/com/blazebit/persistence/integration/datanucleus/DataNucleus51JpaProvider.java
@@ -471,7 +471,7 @@ public boolean supportsSingleValuedAssociationNaturalIdExpressions() {
     }
 
     @Override
-    public boolean needsElementCollectionIdCutoffForCompositeIdOwner() {
+    public boolean needsElementCollectionIdCutoff() {
         return false;
     }
 

diff --git a/...rc/main/java/com/blazebit/persistence/integration/datanucleus/DataNucleusJpaProvider.java b/...rc/main/java/com/blazebit/persistence/integration/datanucleus/DataNucleusJpaProvider.java
@@ -471,7 +471,7 @@ public boolean supportsSingleValuedAssociationNaturalIdExpressions() {
     }
 
     @Override
-    public boolean needsElementCollectionIdCutoffForCompositeIdOwner() {
+    public boolean needsElementCollectionIdCutoff() {
         return false;
     }
 

diff --git a/...rc/main/java/com/blazebit/persistence/integration/eclipselink/EclipseLinkJpaProvider.java b/...rc/main/java/com/blazebit/persistence/integration/eclipselink/EclipseLinkJpaProvider.java
@@ -532,7 +532,7 @@ public boolean supportsSingleValuedAssociationNaturalIdExpressions() {
     }
 
     @Override
-    public boolean needsElementCollectionIdCutoffForCompositeIdOwner() {
+    public boolean needsElementCollectionIdCutoff() {
         return false;
     }
 

diff --git a/...c/main/java/com/blazebit/persistence/integration/hibernate/base/HibernateJpaProvider.java b/...c/main/java/com/blazebit/persistence/integration/hibernate/base/HibernateJpaProvider.java
@@ -236,7 +236,7 @@ public boolean supportsSingleValuedAssociationNaturalIdExpressions() {
     }
 
     @Override
-    public boolean needsElementCollectionIdCutoffForCompositeIdOwner() {
+    public boolean needsElementCollectionIdCutoff() {
         return needsElementCollectionIdCutoffForCompositeIdOwner;
     }
 

diff --git a/...penjpa/src/main/java/com/blazebit/persistence/integration/openjpa/OpenJPAJpaProvider.java b/...penjpa/src/main/java/com/blazebit/persistence/integration/openjpa/OpenJPAJpaProvider.java
@@ -316,7 +316,7 @@ public boolean supportsSingleValuedAssociationNaturalIdExpressions() {
     }
 
     @Override
-    public boolean needsElementCollectionIdCutoffForCompositeIdOwner() {
+    public boolean needsElementCollectionIdCutoff() {
         return false;
     }