From 90b8fcd0b3a996de45ae3b902583bec6e8b8ef5d Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 27 Jun 2019 09:34:18 +0200 Subject: [PATCH] #47 - Refactoring EnversRevisionRepositoryImpl. The refactored version uses a more flexible approach to querying revisions. This will facilitate the necessary changes for #47. It also allows to acquire all required information in one query which at least in theory should be faster. Original pull request: #195. --- .../support/DefaultRevisionMetadata.java | 25 ++- .../support/EnversRevisionRepositoryImpl.java | 206 +++++++----------- .../repository/support/package-info.java | 5 + ...EnversRevisionRepositoryImplUnitTests.java | 103 --------- .../support/RepositoryIntegrationTests.java | 30 ++- 5 files changed, 136 insertions(+), 233 deletions(-) create mode 100644 src/main/java/org/springframework/data/envers/repository/support/package-info.java delete mode 100644 src/test/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImplUnitTests.java diff --git a/src/main/java/org/springframework/data/envers/repository/support/DefaultRevisionMetadata.java b/src/main/java/org/springframework/data/envers/repository/support/DefaultRevisionMetadata.java index 0cb6be0..d0889ac 100755 --- a/src/main/java/org/springframework/data/envers/repository/support/DefaultRevisionMetadata.java +++ b/src/main/java/org/springframework/data/envers/repository/support/DefaultRevisionMetadata.java @@ -23,6 +23,7 @@ import lombok.Value; import java.time.Instant; import java.time.LocalDateTime; import java.time.ZoneOffset; +import java.util.Objects; import java.util.Optional; import org.hibernate.envers.DefaultRevisionEntity; @@ -31,6 +32,9 @@ import org.springframework.data.history.RevisionMetadata; /** * {@link RevisionMetadata} working with a {@link DefaultRevisionEntity}. * + * The entity/delegate itself gets ignored for {@link #equals(Object)} and {@link #hashCode()} since they depend on the + * way they were obtained. + * * @author Oliver Gierke * @author Philip Huegelmeyer * @author Jens Schauder @@ -57,7 +61,6 @@ public class DefaultRevisionMetadata implements RevisionMetadata { return getRevisionInstant().map(instant -> LocalDateTime.ofInstant(instant, ZoneOffset.systemDefault())); } - /* * (non-Javadoc) * @see org.springframework.data.history.RevisionMetadata#getRevisionInstant() @@ -75,4 +78,24 @@ public class DefaultRevisionMetadata implements RevisionMetadata { public T getDelegate() { return (T) entity; } + + @Override + public boolean equals(Object o) { + + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + DefaultRevisionMetadata that = (DefaultRevisionMetadata) o; + return getRevisionNumber().equals(that.getRevisionNumber()) + && getRevisionInstant().equals(that.getRevisionInstant()); + } + + @Override + public int hashCode() { + + return Objects.hash(getRevisionNumber(), getRevisionInstant()); + } } diff --git a/src/main/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImpl.java b/src/main/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImpl.java index 6e27b82..b8a02c3 100755 --- a/src/main/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImpl.java +++ b/src/main/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImpl.java @@ -15,12 +15,9 @@ */ package org.springframework.data.envers.repository.support; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; import javax.persistence.EntityManager; @@ -29,6 +26,10 @@ import org.hibernate.envers.AuditReaderFactory; import org.hibernate.envers.DefaultRevisionEntity; import org.hibernate.envers.RevisionNumber; import org.hibernate.envers.RevisionTimestamp; +import org.hibernate.envers.RevisionType; +import org.hibernate.envers.query.AuditEntity; +import org.hibernate.envers.query.AuditQuery; +import org.hibernate.envers.query.order.AuditOrder; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; @@ -41,7 +42,6 @@ import org.springframework.data.jpa.repository.support.JpaEntityInformation; import org.springframework.data.repository.core.EntityInformation; import org.springframework.data.repository.history.RevisionRepository; import org.springframework.data.repository.history.support.RevisionEntityInformation; -import org.springframework.data.util.StreamUtils; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; @@ -87,23 +87,17 @@ public class EnversRevisionRepositoryImpl> findLastChangeRevision(ID id) { - Class type = entityInformation.getJavaType(); - AuditReader reader = AuditReaderFactory.get(entityManager); + List singleResult = createBaseQuery(id) // + .addOrder(AuditEntity.revisionProperty("timestamp").desc()) // + .setMaxResults(1) // + .getResultList(); - List revisions = getRevisions(id, type, reader); + Assert.state(singleResult.size() <= 1, "We expect at most one result."); - if (revisions.isEmpty()) { - return Optional.empty(); - } - - N latestRevision = (N) revisions.get(revisions.size() - 1); - - Class revisionEntityClass = revisionEntityInformation.getRevisionEntityClass(); - - Object revisionEntity = reader.findRevision(revisionEntityClass, latestRevision); - RevisionMetadata metadata = (RevisionMetadata) getRevisionMetadata(revisionEntity); - - return Optional.of(Revision.of(metadata, reader.find(type, id, latestRevision))); + return singleResult.stream() // + .findFirst() // + .map(QueryResult::new) // + .map(this::createRevision); } /* @@ -111,140 +105,106 @@ public class EnversRevisionRepositoryImpl> findRevision(ID id, N revisionNumber) { Assert.notNull(id, "Identifier must not be null!"); Assert.notNull(revisionNumber, "Revision number must not be null!"); - return getEntityForRevision(revisionNumber, id, AuditReaderFactory.get(entityManager)); + List singleResult = (List) createBaseQuery(id) // + .add(AuditEntity.revisionNumber().eq(revisionNumber)) // + .getResultList(); + + Assert.state(singleResult.size() <= 1, "We expect at most one result."); + + return singleResult.stream() // + .findFirst() // + .map(QueryResult::new) // + .map(this::createRevision); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.history.RevisionRepository#findRevisions(java.io.Serializable) - */ @SuppressWarnings("unchecked") public Revisions findRevisions(ID id) { - Class type = entityInformation.getJavaType(); - AuditReader reader = AuditReaderFactory.get(entityManager); - List revisionNumbers = getRevisions(id, type, reader); + List resultList = createBaseQuery(id) // + .getResultList(); + + List> revisionList = resultList.stream() // + .map(QueryResult::new) // + .map(this::createRevision) // + .collect(Collectors.toList()); + + return Revisions.of(revisionList); - return revisionNumbers.isEmpty() ? Revisions.none() - : getEntitiesForRevisions((List) revisionNumbers, id, reader); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.history.RevisionRepository#findRevisions(java.io.Serializable, org.springframework.data.domain.Pageable) - */ @SuppressWarnings("unchecked") public Page> findRevisions(ID id, Pageable pageable) { + AuditOrder sorting = RevisionSort.getRevisionDirection(pageable.getSort()).isDescending() // + ? AuditEntity.revisionNumber().desc() // + : AuditEntity.revisionNumber().asc(); + + List resultList = createBaseQuery(id) // + .addOrder(sorting) // + .setFirstResult((int) pageable.getOffset()) // + .setMaxResults(pageable.getPageSize()) // + .getResultList(); + + Long count = (Long) createBaseQuery(id) // + .addProjection(AuditEntity.revisionNumber().count()).getSingleResult(); + + List> revisions = resultList.stream() + .map(singleResult -> createRevision(new QueryResult(singleResult))).collect(Collectors.toList()); + + return new PageImpl<>(revisions, pageable, count); + } + + private AuditQuery createBaseQuery(ID id) { + Class type = entityInformation.getJavaType(); AuditReader reader = AuditReaderFactory.get(entityManager); - List revisionNumbers = getRevisions((ID) id, (Class) type, reader); - boolean isDescending = RevisionSort.getRevisionDirection(pageable.getSort()).isDescending(); - if (isDescending) { - Collections.reverse(revisionNumbers); - } - - if ( - pageable.getOffset() >= revisionNumbers.size()) { - return Page.empty(pageable); - } - - long upperBound = pageable.getOffset() + pageable.getPageSize(); - upperBound = upperBound > revisionNumbers.size() ? revisionNumbers.size() : upperBound; - - List subList = revisionNumbers.subList(toInt(pageable.getOffset()), toInt(upperBound)); - Revisions revisions = getEntitiesForRevisions((List) subList, id, reader); - - revisions = isDescending ? revisions.reverse() : revisions; - - return new PageImpl>(revisions.getContent(), pageable, revisionNumbers.size()); + return reader.createQuery() // + .forRevisionsOfEntity(type, false, true) // + .add(AuditEntity.id().eq(id)) ; } - List getRevisions(ID id, Class type, AuditReader reader) { - return reader.getRevisions(type, id); - } + private Revision createRevision(QueryResult queryResult) { - /** - * Returns the entities in the given revisions for the entitiy with the given id. - * - * @param revisionNumbers - * @param id - * @param reader - * @return - */ - @SuppressWarnings("unchecked") - private Revisions getEntitiesForRevisions(List revisionNumbers, ID id, AuditReader reader) { - - Class type = entityInformation.getJavaType(); - Map revisions = new HashMap(revisionNumbers.size()); - - Class revisionEntityClass = revisionEntityInformation.getRevisionEntityClass(); - Map revisionEntities = (Map) reader.findRevisions(revisionEntityClass, - new HashSet(revisionNumbers)); - - for (Number number : revisionNumbers) { - revisions.put((N) number, reader.find(type, type.getName(), id, number, true)); - } - - return Revisions.of(toRevisions(revisions, revisionEntities)); - } - - /** - * Returns an entity in the given revision for the given entity-id. - * - * @param revisionNumber - * @param id - * @param reader - * @return - */ - @SuppressWarnings("unchecked") - private Optional> getEntityForRevision(N revisionNumber, ID id, AuditReader reader) { - - Class type = revisionEntityInformation.getRevisionEntityClass(); - - T revision = (T) reader.findRevision(type, revisionNumber); - Optional entity = Optional.ofNullable(reader.find(entityInformation.getJavaType(), id, revisionNumber)); - - return entity.map(it -> Revision.of((RevisionMetadata) getRevisionMetadata(revision), (T) it)); + return Revision.of(queryResult.createRevisionMetadata(), queryResult.entity); } @SuppressWarnings("unchecked") - private List> toRevisions(Map source, Map revisionEntities) { + private class QueryResult { - return source.entrySet().stream() // - .map(entry -> Revision.of( // - (RevisionMetadata) getRevisionMetadata(revisionEntities.get(entry.getKey())), // - entry.getValue())) // - .sorted() // - .collect(StreamUtils.toUnmodifiableList()); - } + private final T entity; + private final Object metadata; + private final RevisionType revisionType; - /** - * Returns the {@link RevisionMetadata} wrapper depending on the type of the given object. - * - * @param object - * @return - */ - private RevisionMetadata getRevisionMetadata(Object object) { + QueryResult(Object[] data) { - return object instanceof DefaultRevisionEntity // - ? new DefaultRevisionMetadata((DefaultRevisionEntity) object) // - : new AnnotationRevisionMetadata(object, RevisionNumber.class, RevisionTimestamp.class); - } + Assert.notNull(data, "Data must not be null"); + Assert.isTrue( // + data.length == 3, // + () -> String.format("Data must have length three, but has length %d.", data.length)); + Assert.isTrue( // + data[2] instanceof RevisionType, // + () -> String.format("The third array element must be of type Revision type, but is of type %s", + data[2].getClass())); - private static int toInt(long value) { - - if (value > Integer.MAX_VALUE) { - throw new IllegalStateException(String.format("%s can't be mapped to an integer, too large!", value)); + entity = (T) data[0]; + metadata = data[1]; + revisionType = (RevisionType) data[2]; } - return Long.valueOf(value).intValue(); + RevisionMetadata createRevisionMetadata() { + + return metadata instanceof DefaultRevisionEntity // + ? (RevisionMetadata) new DefaultRevisionMetadata((DefaultRevisionEntity) metadata) // + : new AnnotationRevisionMetadata<>(metadata, RevisionNumber.class, RevisionTimestamp.class); + } } + } diff --git a/src/main/java/org/springframework/data/envers/repository/support/package-info.java b/src/main/java/org/springframework/data/envers/repository/support/package-info.java new file mode 100644 index 0000000..dd135fd --- /dev/null +++ b/src/main/java/org/springframework/data/envers/repository/support/package-info.java @@ -0,0 +1,5 @@ +/** + * Spring Data JPA specific converter infrastructure. + */ +@org.springframework.lang.NonNullApi +package org.springframework.data.envers.repository.support; diff --git a/src/test/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImplUnitTests.java b/src/test/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImplUnitTests.java deleted file mode 100644 index 437560c..0000000 --- a/src/test/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImplUnitTests.java +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright 2018-2020 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.envers.repository.support; - -import static org.mockito.Mockito.*; - -import java.util.Collections; -import java.util.List; - -import javax.persistence.EntityManager; - -import org.hibernate.HibernateException; -import org.hibernate.Session; -import org.hibernate.engine.spi.SessionImplementor; -import org.hibernate.envers.AuditReader; -import org.hibernate.envers.boot.internal.EnversService; -import org.hibernate.service.Service; -import org.junit.Before; -import org.junit.Test; - -import org.springframework.data.domain.PageRequest; -import org.springframework.data.jpa.repository.support.JpaEntityInformation; -import org.springframework.data.repository.history.support.RevisionEntityInformation; - -/** - * Unit tests for EversRevisionRepositoryImpl. - * - * @author Jens Schauder - */ -public class EnversRevisionRepositoryImplUnitTests { - - private static final int NON_EXISTING_ID = -999; - - JpaEntityInformation entityInformation = mock(JpaEntityInformation.class); - RevisionEntityInformation revisionEntityInformation = mock(RevisionEntityInformation.class); - SessionImplementor session = mock(SessionImplementor.class, RETURNS_DEEP_STUBS); - EnversService enversService = mock(EnversService.class, RETURNS_DEEP_STUBS); - EntityManager entityManager = mock(EntityManager.class); - - @Before - public void mockHibernateInfrastructure() { - - when(entityInformation.getJavaType()).thenReturn((Class) DummyEntity.class); - - when(enversService.getEntitiesConfigurations().isVersioned(any(String.class))).thenReturn(true); - - when(session.isOpen()).thenReturn(true); - when((Service) session.getFactory().getServiceRegistry().getService(EnversService.class)).thenReturn(enversService); - - when(entityManager.getDelegate()).thenReturn(session); - } - - @Test // #146 - public void findRevisionShortCircuitsOnEmptyRevisionList() { - - failOnEmptyRevisions(); - - EnversRevisionRepositoryImplUnderTest repository = new EnversRevisionRepositoryImplUnderTest<>( - entityInformation, revisionEntityInformation, entityManager); - - repository.findRevisions(-999, PageRequest.of(0, 5)); - } - - private void failOnEmptyRevisions() { - - // simulate failure to query with empty revisions list as Postgres does. - when(enversService.getRevisionInfoQueryCreator().getRevisionsQuery(any(Session.class), eq(Collections.emptySet())) - .getResultList()).thenThrow(HibernateException.class); - } - - /** - * An extension for the {@link EnversRevisionRepositoryImpl} that skips accessing the AuditReader and always returns - * an empty List. - */ - private class EnversRevisionRepositoryImplUnderTest> - extends EnversRevisionRepositoryImpl { - - EnversRevisionRepositoryImplUnderTest(JpaEntityInformation entityInformation, - RevisionEntityInformation revisionEntityInformation, EntityManager entityManager) { - super(entityInformation, revisionEntityInformation, entityManager); - } - - @Override - List getRevisions(ID id, Class type, AuditReader reader) { - return Collections.emptyList(); - } - } - - private static class DummyEntity {} -} diff --git a/src/test/java/org/springframework/data/envers/repository/support/RepositoryIntegrationTests.java b/src/test/java/org/springframework/data/envers/repository/support/RepositoryIntegrationTests.java index a7a2229..5517f5d 100755 --- a/src/test/java/org/springframework/data/envers/repository/support/RepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/envers/repository/support/RepositoryIntegrationTests.java @@ -109,10 +109,20 @@ public class RepositoryIntegrationTests { } @Test // #1 + public void returnsEmptyLastRevisionForUnrevisionedEntity() { + assertThat(countryRepository.findLastChangeRevision(100L)).isEmpty(); + } + + @Test // #47 public void returnsEmptyRevisionsForUnrevisionedEntity() { assertThat(countryRepository.findRevisions(100L)).isEmpty(); } + @Test // #47 + public void returnsEmptyRevisionForUnrevisionedEntity() { + assertThat(countryRepository.findRevision(100L, 23)).isEmpty(); + } + @Test // #31 public void returnsParticularRevisionForAnEntity() { @@ -183,7 +193,8 @@ public class RepositoryIntegrationTests { } @Test // #146 - public void shortCurcuitingWhenOffsetIsToLarge() { + public void shortCircuitingWhenOffsetIsToLarge() { + Country de = new Country(); de.code = "de"; de.name = "Deutschland"; @@ -192,14 +203,21 @@ public class RepositoryIntegrationTests { countryRepository.delete(de); - check(de, 0, 1); - check(de, 1, 1); - check(de, 2, 0); + check(de.id, 0, 1, 2); + check(de.id, 1, 1, 2); + check(de.id, 2, 0, 2); } - void check(Country de, int page, int expectedSize) { + @Test // #47 + public void paginationWithEmptyResult() { - Page> revisions = countryRepository.findRevisions(de.id, PageRequest.of(page, 1)); + check(23L, 0, 0, 0); + } + + void check(Long id, int page, int expectedSize, int expectedTotalSize) { + + Page> revisions = countryRepository.findRevisions(id, PageRequest.of(page, 1)); assertThat(revisions).hasSize(expectedSize); + assertThat(revisions.getTotalElements()).isEqualTo(expectedTotalSize); } }