Skip to content

Commit

Permalink
Fix updatable entity views issue regarding wrong initial state, suppo…
Browse files Browse the repository at this point in the history
…rt different fetch types for updatable attributes, Fix mapped superclass issue
  • Loading branch information
beikov committed May 8, 2018
1 parent 44b227f commit 4181f1b
Show file tree
Hide file tree
Showing 14 changed files with 155 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,13 @@ public Collection<Object> getRemovedObjects() {

@Override
public Collection<Object> getAddedObjects(C collection) {
return (Collection<Object>) elements;
List<Object> objects = new ArrayList<>(elements.size());
for (Object element : elements) {
if (!collection.contains(element)) {
objects.add(element);
}
}
return objects;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ public Collection<Object> getRemovedObjects() {

@Override
public Collection<Object> getAddedObjects(C collection) {
if (collection.contains(element)) {
return Collections.emptyList();
}
return Collections.<Object>singleton(element);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,13 @@ public Collection<Object> getRemovedObjects() {

@Override
public Collection<Object> getAddedObjects(C collection) {
return (Collection<Object>) elements;
List<Object> objects = new ArrayList<>(elements.size());
for (Object element : elements) {
if (!collection.contains(element)) {
objects.add(element);
}
}
return objects;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ protected void populateResult(Map<Object, TuplePromise> correlationValues, Objec
}

if (element[1] != null) {
result.add(element[1]);
add(result, element[1]);
}
}

Expand All @@ -73,6 +73,14 @@ protected void populateResult(Map<Object, TuplePromise> correlationValues, Objec
}
}

private void add(Collection<Object> result, Object o) {
if (recording) {
((RecordingCollection<?, Object>) result).getDelegate().add(o);
} else {
result.add(o);
}
}

@Override
public Object copy(Object o) {
return createCollection((Collection<? extends Object>) o);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ protected void populateResult(boolean usesViewRoot, Map<Object, Map<Object, Tupl
}

if (element[2] != null) {
result.add(element[2]);
add(result, element[2]);
}
}
} else {
Expand All @@ -82,7 +82,7 @@ protected void populateResult(boolean usesViewRoot, Map<Object, Map<Object, Tupl
}

if (element[1] != null) {
result.add(element[1]);
add(result, element[1]);
}
}
}
Expand All @@ -95,6 +95,14 @@ protected void populateResult(boolean usesViewRoot, Map<Object, Map<Object, Tupl
}
}

private void add(Collection<Object> result, Object o) {
if (recording) {
((RecordingCollection<?, Object>) result).getDelegate().add(o);
} else {
result.add(o);
}
}

@Override
public Object copy(Object o) {
return createCollection((Collection<? extends Object>) o);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,10 @@ public FetchGraphNode<?> mergeWith(List<CollectionAttributeFlusher<E, V>> fetchG
fetchChanged |= this.fetch != node.fetch;
if (node.nestedGraphNode != null) {
if (node.nestedGraphNode instanceof CollectionElementFetchGraphNode) {
nestedFlushers.add(((CollectionElementFetchGraphNode) node.nestedGraphNode).nestedGraphNode);
CollectionElementFetchGraphNode collectionElementFetchGraphNode = (CollectionElementFetchGraphNode) node.nestedGraphNode;
if (collectionElementFetchGraphNode.nestedGraphNode != null) {
nestedFlushers.add(collectionElementFetchGraphNode.nestedGraphNode);
}
} else {
nestedFlushers.add(node.nestedGraphNode);
}
Expand Down Expand Up @@ -238,7 +241,7 @@ public void flushQuery(UpdateContext context, String parameterPrefix, Query quer
Map<Object, Object> removed;

if (recordingCollection.hasActions()) {
Map<Object, Object>[] addedAndRemoved = getAddedAndRemovedElements(recordingCollection, (List<CollectionAction<Collection<?>>>) (List<?>) recordingCollection.resetActions(context));
Map<Object, Object>[] addedAndRemoved = getAddedAndRemovedElements(recordingCollection, context);
added = addedAndRemoved[0];
removed = addedAndRemoved[1];
} else {
Expand Down Expand Up @@ -269,7 +272,7 @@ public boolean flushEntity(UpdateContext context, E entity, Object view, V value
Map<Object, Object> added;
Map<Object, Object> removed;
if (recordingCollection.hasActions()) {
Map<Object, Object>[] addedAndRemoved = getAddedAndRemovedElements(recordingCollection, (List<CollectionAction<Collection<?>>>) (List<?>) recordingCollection.resetActions(context));
Map<Object, Object>[] addedAndRemoved = getAddedAndRemovedElements(recordingCollection, context);
added = addedAndRemoved[0];
removed = addedAndRemoved[1];
} else {
Expand Down Expand Up @@ -920,12 +923,14 @@ protected DirtyAttributeFlusher<CollectionAttributeFlusher<E, V>, E, V> determin
}

if (inverseFlusher != null) {
Map<Object, Object>[] addedAndRemoved;
// Always reset the actions as that indicates changes
if (current instanceof RecordingCollection<?, ?>) {
((RecordingCollection<?, ?>) current).resetActions(context);
addedAndRemoved = getAddedAndRemovedElements((RecordingCollection<?, ?>) current, context);
} else {
addedAndRemoved = getAddedAndRemovedElements(current, collectionActions);
}
// Inverse collections must convert collection actions to element flush actions
Map<Object, Object>[] addedAndRemoved = getAddedAndRemovedElements(current, collectionActions);
Map<Object, Object> added = addedAndRemoved[0];
Map<Object, Object> removed = addedAndRemoved[1];
List<CollectionElementAttributeFlusher<E, V>> elementFlushers = getInverseElementFlushersForActions(context, current, added, removed);
Expand Down Expand Up @@ -1417,7 +1422,7 @@ protected DirtyAttributeFlusher<CollectionAttributeFlusher<E, V>, E, V> getDirty
}
} else if (inverseFlusher != null) {
// Inverse collections must convert collection actions to element flush actions
Map<Object, Object>[] addedAndRemoved = getAddedAndRemovedElements(collection, (List<CollectionAction<Collection<?>>>) (List<?>) collection.resetActions(context));
Map<Object, Object>[] addedAndRemoved = getAddedAndRemovedElements(collection, context);
Map<Object, Object> added = addedAndRemoved[0];
Map<Object, Object> removed = addedAndRemoved[1];
List<CollectionElementAttributeFlusher<E, V>> elementFlushers = getInverseElementFlushersForActions(context, collection, added, removed);
Expand Down Expand Up @@ -1454,6 +1459,29 @@ protected DirtyAttributeFlusher<CollectionAttributeFlusher<E, V>, E, V> getDirty
return null;
}

@SuppressWarnings("unchecked")
private Map<Object, Object>[] getAddedAndRemovedElements(RecordingCollection<?, ?> collection, UpdateContext context) {
List<? extends CollectionAction<?>> collectionActions = collection.resetActions(context);
Map<Object, Object> added = new IdentityHashMap<>();
Map<Object, Object> removed = new IdentityHashMap<>();
for (CollectionAction<? extends Collection<?>> a : collectionActions) {
Collection<Object> addedObjects = a.getAddedObjects();
Collection<Object> removedObjects = a.getRemovedObjects();

for (Object addedObject : addedObjects) {
removed.remove(addedObject);
}
for (Object removedObject : removedObjects) {
added.remove(removedObject);
removed.put(removedObject, removedObject);
}
for (Object addedObject : addedObjects) {
added.put(addedObject, addedObject);
}
}
return new Map[]{ added, removed };
}

@SuppressWarnings("unchecked")
private Map<Object, Object>[] getAddedAndRemovedElements(Collection<?> collection, List<CollectionAction<Collection<?>>> collectionActions) {
Map<Object, Object> added = new IdentityHashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ public void flushQuery(UpdateContext context, String parameterPrefix, Query quer
final DirtyAttributeFlusher<?, Object, Object> flusher = flushers[index];
Object newInitialValue = flusher.cloneDeep(value, initialState[index], state[index]);
flusher.flushQuery(context, parameterPrefix, query, value, state[index]);
initialState[i] = flusher.getNewInitialValue(context, newInitialValue, state[index]);
initialState[index] = flusher.getNewInitialValue(context, newInitialValue, state[index]);
}
} else {
for (int i = 0; i < deferredFlushers.size(); i++) {
Expand Down Expand Up @@ -612,7 +612,7 @@ private void deferredFlushEntity(UpdateContext context, Object entity, MutableSt
final DirtyAttributeFlusher<?, Object, Object> flusher = flushers[index];
Object newInitialValue = flusher.cloneDeep(updatableProxy, initialState[index], state[index]);
flusher.flushEntity(context, entity, updatableProxy, state[index], null);
initialState[i] = flusher.getNewInitialValue(context, newInitialValue, state[index]);
initialState[index] = flusher.getNewInitialValue(context, newInitialValue, state[index]);
}
} else {
for (int i = 0; i < deferredFlushers.size(); i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import com.blazebit.persistence.criteria.BlazeCriteriaBuilder;
import com.blazebit.persistence.criteria.BlazeCriteriaQuery;
import com.blazebit.persistence.criteria.impl.BlazeCriteria;
import com.blazebit.persistence.parser.EntityMetamodel;
import com.blazebit.persistence.spi.ExtendedManagedType;
import com.blazebit.persistence.spring.data.base.query.KeysetAwarePageImpl;
import com.blazebit.persistence.spring.data.repository.EntityViewRepository;
import com.blazebit.persistence.spring.data.repository.EntityViewSpecificationExecutor;
Expand Down Expand Up @@ -53,7 +55,6 @@
import javax.persistence.criteria.Order;
import javax.persistence.criteria.Predicate;
import javax.persistence.criteria.Root;
import javax.persistence.metamodel.EntityType;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -81,6 +82,7 @@ public class EntityViewAwareRepositoryImpl<V, E, ID extends Serializable> implem
private final CriteriaBuilderFactory cbf;
private final EntityViewManager evm;
private final Class<V> entityViewClass;
private final String idAttributeName;

private EntityViewAwareCrudMethodMetadata metadata;

Expand All @@ -90,6 +92,7 @@ public EntityViewAwareRepositoryImpl(JpaEntityInformation<E, ?> entityInformatio
this.cbf = cbf;
this.evm = evm;
this.entityViewClass = entityViewClass;
this.idAttributeName = getIdAttribute(getDomainClass());
}

public void setRepositoryMethodMetadata(EntityViewAwareCrudMethodMetadata crudMethodMetadata) {
Expand Down Expand Up @@ -308,7 +311,7 @@ public V findOne(ID id) {
Assert.notNull(id, ID_MUST_NOT_BE_NULL);

CriteriaBuilder<?> cb = cbf.create(entityManager, getDomainClass())
.where(getIdAttribute()).eq(id);
.where(idAttributeName).eq(id);
String[] fetches = EMPTY;
if (metadata != null && metadata.getEntityGraph() != null && (fetches = metadata.getEntityGraph().attributePaths()).length != 0) {
cb.fetch(fetches);
Expand All @@ -323,7 +326,11 @@ public V findOne(ID id) {

applyQueryHints(findOneQuery, fetches.length == 0);

return findOneQuery.getSingleResult();
try {
return findOneQuery.getSingleResult();
} catch (NoResultException e) {
return null;
}
}

@Override
Expand All @@ -340,15 +347,21 @@ public boolean existsById(ID id) {
public boolean exists(ID id) {
Assert.notNull(id, ID_MUST_NOT_BE_NULL);

TypedQuery<Long> existsQuery = cbf.create(entityManager, Long.class)
TypedQuery<Object> existsQuery = cbf.create(entityManager, Object.class)
.from(getDomainClass())
.select("COUNT(*)")
.where(getIdAttribute()).eq(id)
// Empty string because SQLServer can't interpret a number properly when using TOP clause
.select("''")
.where(idAttributeName).eq(id)
.setMaxResults(1)
.getQuery();

applyRepositoryMethodMetadata(existsQuery, true);

return existsQuery.getSingleResult() > 0;
try {
return !existsQuery.getResultList().isEmpty();
} catch (NoResultException e) {
return false;
}
}

@Override
Expand All @@ -369,7 +382,7 @@ public List<V> findAll(Iterable<ID> idIterable) {
idList.add(id);
}
CriteriaBuilder<?> cb = cbf.create(entityManager, getDomainClass())
.where(getIdAttribute()).in(idList);
.where(idAttributeName).in(idList);

String[] fetches = EMPTY;
if (metadata != null && metadata.getEntityGraph() != null && (fetches = metadata.getEntityGraph().attributePaths()).length != 0) {
Expand All @@ -389,12 +402,10 @@ public List<V> findAll(Iterable<ID> idIterable) {
}

private String getIdAttribute(Class<?> entityClass) {
EntityType<?> entityType = entityManager.getMetamodel().entity(entityClass);
return entityType.getDeclaredId(entityType.getIdType().getJavaType()).getName();
}

private String getIdAttribute() {
return getIdAttribute(getDomainClass());
return cbf.getService(EntityMetamodel.class)
.getManagedType(ExtendedManagedType.class, entityClass)
.getIdAttribute()
.getName();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.blazebit.persistence.spring.data.testsuite;

import com.blazebit.persistence.spring.data.testsuite.config.CustomLocalContainerEntityManagerFactoryBean;
import com.blazebit.persistence.spring.data.testsuite.entity.Document;
import com.blazebit.persistence.spring.data.testsuite.entity.Person;
import com.blazebit.persistence.testsuite.base.AbstractPersistenceTest;
Expand Down Expand Up @@ -49,12 +50,23 @@ protected void configurePersistenceUnitInfo(MutablePersistenceUnitInfo persisten

@Before
public void setUpContext() throws Exception {
// We have to close the EM and EMF constructed by the abstraction
cleanDatabase();
this.em.getTransaction().rollback();
this.em.close();
this.emf.close();
this.emf = null;
this.em = null;
this.cbf = null;
this.jpaProvider = null;
this.dbmsDialect = null;

//this is where the magic happens, we actually do "by hand" what the spring runner would do for us,
// read the JavaDoc for the class bellow to know exactly what it does, the method names are quite accurate though
CustomLocalContainerEntityManagerFactoryBean.properties = createProperties("none");
testContextManager = new TestContextManager(getClass());
testContextManager.prepareTestInstance(this);
testContextManager.registerTestExecutionListeners(new DirtiesContextTestExecutionListener());
cleanDatabase();
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import com.blazebit.persistence.testsuite.base.jpa.category.NoDatanucleus;
import com.blazebit.persistence.testsuite.base.jpa.category.NoEclipselink;
import com.blazebit.persistence.testsuite.base.jpa.category.NoHibernate42;
import com.blazebit.persistence.testsuite.base.jpa.category.NoMSSQL;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -266,8 +265,6 @@ public void testFindByNameIn() {
}

@Test
// Since only Hibernate 5.1 fixed a bug in the SQLServer dialect regarding pagination, we will skip this test for MSSQL
@Category({ NoMSSQL.class })
public void testFindByNameInPaginated() {
// ignored with EclipseLink due to IN collection rendering bug
Assume.assumeFalse(isEntityRepository() && isEclipseLink());
Expand Down Expand Up @@ -297,8 +294,6 @@ public void testFindByNameInPaginated() {
}

@Test
// Since only Hibernate 5.1 fixed a bug in the SQLServer dialect regarding pagination, we will skip this test for MSSQL
@Category({ NoMSSQL.class })
public void testFindByNameInKeysetPaginated() {
// ignored with EclipseLink due to IN collection rendering bug
Assume.assumeFalse(isEntityRepository() && isEclipseLink());
Expand Down Expand Up @@ -380,8 +375,6 @@ public void testFindByAgeGreaterThanEqual() {
}

@Test
// Since only Hibernate 5.1 fixed a bug in the SQLServer dialect regarding pagination, we will skip this test for MSSQL
@Category({ NoMSSQL.class })
public void testFindSliceByAgeGreaterThanEqual() {
// Given
final Document d1 = createDocument("d1", null, 3L, null);
Expand All @@ -400,8 +393,6 @@ public void testFindSliceByAgeGreaterThanEqual() {
}

@Test
// Since only Hibernate 5.1 fixed a bug in the SQLServer dialect regarding pagination, we will skip this test for MSSQL
@Category({ NoMSSQL.class })
public void testFindFirstByOrderByNameAsc() {
// Given
final Document d3 = createDocument("d3");
Expand Down Expand Up @@ -476,8 +467,6 @@ public Predicate toPredicate(Root<Document> root, CriteriaQuery<?> criteriaQuery
}

@Test
// Since only Hibernate 5.1 fixed a bug in the SQLServer dialect regarding pagination, we will skip this test for MSSQL
@Category({ NoMSSQL.class })
public void testFindAllBySpecPageable() {
// Given
final Document d4 = createDocument("d4", null, 2L, null);
Expand Down
Loading

0 comments on commit 4181f1b

Please sign in to comment.