fix: findAllVersions should return provided entity when it is a versi…

…on series of 1 - Fixes #2039
paulcwarren · Jul 30, 2024 · 009cd3d · 009cd3d
1 parent 9d3158a
commit 009cd3d
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 28 deletions.
diff --git a/...a/src/main/java/org/springframework/versions/impl/LockingAndVersioningRepositoryImpl.java b/...a/src/main/java/org/springframework/versions/impl/LockingAndVersioningRepositoryImpl.java
@@ -5,11 +5,7 @@
 import java.io.Serializable;
 import java.lang.reflect.Field;
 import java.security.Principal;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
+import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 
 import jakarta.persistence.EntityManager;
@@ -24,8 +20,6 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.content.commons.utils.BeanUtils;
 import org.springframework.data.domain.Sort;
-import org.springframework.data.jpa.repository.support.JpaEntityInformation;
-import org.springframework.data.repository.CrudRepository;
 import org.springframework.data.repository.core.EntityInformation;
 import org.springframework.security.core.Authentication;
 import org.springframework.stereotype.Repository;
@@ -185,7 +179,7 @@ public <S extends T> S workingCopy(S currentVersion) {
         }
 
         S ancestorRoot;
-        if (isAnestralRoot(currentVersion)) {
+        if (isAncestralRoot(currentVersion)) {
             currentVersion = (S)versioner.establishAncestralRoot(currentVersion);
             em.merge(currentVersion);
             ancestorRoot = currentVersion;
@@ -240,7 +234,7 @@ public <S extends T> S version(S currentVersion, VersionInfo info) {
         if (!isPrivateWorkingCopy(currentVersion)) {
 
             S ancestorRoot;
-            if (isAnestralRoot(currentVersion)) {
+            if (isAncestralRoot(currentVersion)) {
                 currentVersion = (S) versioner.establishAncestralRoot(currentVersion);
                 ancestorRoot = currentVersion;
             }
@@ -311,6 +305,10 @@ public <S extends T> List<S> findAllVersions(S entity) {
     @Override
     public <S extends T> List<S> findAllVersions(S entity, Sort sort) {
 
+        if (isAncestralRoot(entity)) {
+            return Collections.singletonList(entity);
+        }
+
         StringBuilder builder = new StringBuilder();
         if (sort.isSorted()) {
             builder.append("order by ");
@@ -435,7 +433,7 @@ protected <T> boolean isHead(T entity) {
         return isHead;
     }
 
-    protected <S extends T> boolean isAnestralRoot(S entity) {
+    protected <S extends T> boolean isAncestralRoot(S entity) {
         boolean isAncestralRoot = false;
         if (BeanUtils.hasFieldWithAnnotation(entity, AncestorRootId.class)) {
             return BeanUtils.getFieldWithAnnotation(entity, AncestorRootId.class) == null;

diff --git a/.../test/java/org/springframework/versions/impl/JpaLockingAndVersioningRepositoryImplIT.java b/.../test/java/org/springframework/versions/impl/JpaLockingAndVersioningRepositoryImplIT.java
@@ -1,16 +1,13 @@
 package org.springframework.versions.impl;
 
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.BeforeEach;
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.Context;
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.Describe;
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.It;
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.JustBeforeEach;
+import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.*;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasItems;
 import static org.hamcrest.beans.HasPropertyWithValue.hasProperty;
 import static org.hamcrest.number.OrderingComparison.greaterThan;
 import static org.junit.Assert.fail;
@@ -419,22 +416,13 @@ public class JpaLockingAndVersioningRepositoryImplIT {
                     It("should return the version series", () -> {
                         List<TestEntity> results = repo.findAllVersions(e1, Sort.by(Order.desc("id")));
                         assertThat(results.size(), is(2));
-                        assertThat(results, Matchers.hasItems(hasProperty("xid", is(e1.getXid())), hasProperty("xid", is(e1v11.getXid()))));
+                        assertThat(results, hasItems(hasProperty("xid", is(e1.getXid())), hasProperty("xid", is(e1v11.getXid()))));
                     });
 
                     It("should return the ordered version series", () -> {
                         List<TestEntity> results = repo.findAllVersions(e1, Sort.by(Order.desc("id")));
                         assertThat(results.size(), is(2));
-                        assertThat(results, Matchers.hasItems(hasProperty("xid", is(e1.getXid())), hasProperty("xid", is(e1v11.getXid()))));
-
-                        // is ordered
-                        {
-                            long lastId = 0;
-                            for (int i=results.size() - 1; i >= 0; i--) {
-                                assertThat(results.get(i).getXid(), is(greaterThan(lastId)));
-                                lastId = results.get(i).getXid();
-                            }
-                        }
+                        assertThat(results, Matchers.contains(hasProperty("xid", is(e1v11.getXid())), hasProperty("xid", is(e1.getXid()))));
                     });
                 });
 
@@ -459,14 +447,14 @@ public class JpaLockingAndVersioningRepositoryImplIT {
 
                     It("should return only the latest version of the entities", () -> {
                         List<TestEntity> results = repo.findAllVersionsLatest((Class<TestEntity>) e1.getClass());
-                        assertThat(results, Matchers.hasItems(
+                        assertThat(results, hasItems(
                                 hasProperty("xid", is(e1v11.getXid())),
                                 hasProperty("xid", is(e2v2.getXid())),
                                 not(hasProperty("xid", is(e3wc.getXid())))
                         ));
 
                         results = repo.findAllVersionsLatest(TestEntity.class);
-                        assertThat(results, Matchers.hasItems(
+                        assertThat(results, hasItems(
                                 hasProperty("xid", is(e1v11.getXid())),
                                 hasProperty("xid", is(e2v2.getXid())),
                                 not(hasProperty("xid", is(e3wc.getXid())))
@@ -861,6 +849,21 @@ public class JpaLockingAndVersioningRepositoryImplIT {
                         });
                     });
                 });
+
+                Context("Issue #2039", () -> {
+                    BeforeEach(() -> {
+                        setupSecurityContext("some-principal", true);
+
+                        e1 = repo.save(new TestEntity());
+                        e2 = repo.save(new TestEntity());
+                        e3 = repo.save(new TestEntity());
+                    });
+
+                    It("should return the provided entity", () -> {
+                        List<TestEntity> results = repo.findAllVersions(e1, Sort.by(Order.desc("id")));
+                        assertThat(results.size(), is(1));
+                    });
+                });
             });
         });
     }