SHIBUI-2050

Fixing test issues
TIER · Sep 2, 2021 · 7ead267 · 7ead267
1 parent 813e54d
commit 7ead267
Show file tree

Hide file tree

Showing 9 changed files with 98 additions and 91 deletions.
diff --git a/...ain/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/SvnMetadataResource.java b/...ain/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/SvnMetadataResource.java
@@ -16,19 +16,21 @@
 @EqualsAndHashCode
 public class SvnMetadataResource {
 
-    private String repositoryURL;
+    private String password;
 
-    private String workingCopyDirectory;
+    private String proxyHost;
 
-    private String resourceFile;
+    private String proxyPassword;
 
-    private String username;
+    private String proxyPort;
 
-    private String password;
+    private String proxyUserName;
 
-    private String proxyHost;
+    private String repositoryURL;
 
-    private String proxyUserName;
+    private String resourceFile;
 
-    private String proxyPassword;
-}
+    private String username;
+
+    private String workingCopyDirectory;
+}
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java
@@ -78,6 +78,7 @@ public void registerLoader(ILazyLoaderHelper lazyLoaderHelper) {
     public Set<Ownership> getOwnedItems() {
         if (lazyLoaderHelper != null) {
             lazyLoaderHelper.loadOwnedItems(this);
+            lazyLoaderHelper = null;
         }
         return ownedItems;
     }

diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java
@@ -71,7 +71,6 @@ public class User extends AbstractAuditable implements Owner, Ownable {
     private Set<Role> roles = new HashSet<>();
 
     @EqualsAndHashCode.Exclude
-    // @JsonIgnore
     @Transient
     private Set<Group> userGroups = new HashSet<>();
 
@@ -130,6 +129,7 @@ public String getRole() {
     public Set<Group> getUserGroups() {
         if (lazyLoaderHelper != null) {
             lazyLoaderHelper.loadGroups(this);
+            lazyLoaderHelper = null;
         }
         return userGroups;
     }

diff --git a/...nternet2/tier/shibboleth/admin/ui/security/model/listener/GroupUpdatedEntityListener.java b/...nternet2/tier/shibboleth/admin/ui/security/model/listener/GroupUpdatedEntityListener.java
@@ -4,13 +4,16 @@
 
 import javax.persistence.PostLoad;
 import javax.persistence.PostPersist;
+import javax.persistence.PostRemove;
 import javax.persistence.PostUpdate;
 
 import org.springframework.beans.factory.annotation.Autowired;
 
 import edu.internet2.tier.shibboleth.admin.ui.security.model.Group;
 import edu.internet2.tier.shibboleth.admin.ui.security.model.Ownership;
 import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
 
 public class GroupUpdatedEntityListener implements ILazyLoaderHelper {
     private static OwnershipRepository ownershipRepository;
@@ -26,17 +29,19 @@ public void init(OwnershipRepository repo) {
     @PostPersist
     @PostUpdate
     @PostLoad
+    @PostRemove
     public synchronized void groupSavedOrFetched(Group group) {
         // Because of the JPA spec, the listener can't do queries in the callback, so we force lazy loading through
         // another callback to this at the time that the owned items are needed
         group.registerLoader(this);
     }
 
     @Override
+    @Transactional(propagation = Propagation.REQUIRES_NEW)
     public void loadOwnedItems(Group group) {
-      group.registerLoader(null); // once loaded, remove the helper from the group
       Set<Ownership> ownedItems = ownershipRepository.findAllByOwner(group);
-      group.setOwnedItems(ownedItems);        
+      group.setOwnedItems(ownedItems);
+      group.registerLoader(null); // once loaded, remove the helper from the group
     }
 
 }
diff --git a/...internet2/tier/shibboleth/admin/ui/security/model/listener/UserUpdatedEntityListener.java b/...internet2/tier/shibboleth/admin/ui/security/model/listener/UserUpdatedEntityListener.java
@@ -1,21 +1,20 @@
 package edu.internet2.tier.shibboleth.admin.ui.security.model.listener;
 
-import java.util.HashSet;
-import java.util.Set;
-
-import javax.persistence.PostLoad;
-import javax.persistence.PostPersist;
-import javax.persistence.PostUpdate;
-
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.transaction.annotation.Propagation;
-import org.springframework.transaction.annotation.Transactional;
-
 import edu.internet2.tier.shibboleth.admin.ui.security.model.Group;
 import edu.internet2.tier.shibboleth.admin.ui.security.model.Ownership;
 import edu.internet2.tier.shibboleth.admin.ui.security.model.User;
 import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository;
 import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import javax.persistence.PostLoad;
+import javax.persistence.PostPersist;
+import javax.persistence.PostRemove;
+import javax.persistence.PostUpdate;
+import java.util.HashSet;
+import java.util.Set;
 
 public class UserUpdatedEntityListener implements ILazyLoaderHelper {
     private static GroupsRepository groupRepository;
@@ -33,15 +32,16 @@ public void init(OwnershipRepository repo, GroupsRepository groupRepo) {
     @PostPersist
     @PostUpdate
     @PostLoad
-    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    @PostRemove
     public synchronized void userSavedOrFetched(User user) {
         // Because of the JPA spec, the listener can't do queries in the callback, so we force lazy loading through
         // another callback to this at the time that the groups are needed
         user.registerLoader(this);
     }
-
+
+    @Override
+    @Transactional(propagation = Propagation.REQUIRES_NEW)
     public void loadGroups(User user) {
-        user.setLazyLoaderHelper(null);
         Set<Ownership> ownerships = ownershipRepository.findAllGroupsForUser(user.getUsername());
         HashSet<Group> groups = new HashSet<>();
         final boolean isAdmin = user.getRole().equals("ROLE_ADMIN");
@@ -52,5 +52,6 @@ public void loadGroups(User user) {
             groups.add(userGroup);
         });
         user.setGroups(groups);
+        user.setLazyLoaderHelper(null);
     }
 }
diff --git a/...end/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/AbstractBaseDataJpaTest.groovy b/...end/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/AbstractBaseDataJpaTest.groovy
@@ -15,11 +15,16 @@ import org.springframework.test.context.ContextConfiguration
 import org.springframework.transaction.annotation.Transactional
 import spock.lang.Specification
 
+import javax.persistence.EntityManager
+
 @DataJpaTest
 @ContextConfiguration(classes = [BaseDataJpaTestConfiguration])
 @EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"])
 @EntityScan("edu.internet2.tier.shibboleth.admin.ui")
 abstract class AbstractBaseDataJpaTest extends Specification {
+    @Autowired
+    EntityManager entityManager
+
     @Autowired
     GroupServiceForTesting groupService
 
@@ -65,6 +70,10 @@ abstract class AbstractBaseDataJpaTest extends Specification {
         createAdminUser()
     }
 
+    def cleanup() {
+        entityManager.clear()
+    }
+
     protected createAdminUser() {
         if (userRepository.findByUsername("admin").isEmpty()) {
             Optional<Role> adminRole = roleRepository.findByName("ROLE_ADMIN")

diff --git a/...est/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy b/...est/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy
@@ -8,22 +8,19 @@ import edu.internet2.tier.shibboleth.admin.ui.util.WithMockAdmin
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.transaction.annotation.Transactional
 
-import javax.persistence.EntityManager
-
 class FilterRepositoryTests extends AbstractBaseDataJpaTest {
     @Autowired
     FilterRepository filterRepository
-
-    @Autowired
-    EntityManager entityManager
 
     @Transactional
     def setup() {
         filterRepository.deleteAll()
     }
 
-    @WithMockAdmin
-    @Transactional
+    def cleanup() {
+        filterRepository.deleteAll()
+    }
+
     def "EntityAttributesFilter hashcode works as desired"() {
         given:
         def entityAttributesFilterJson = '''{
@@ -58,16 +55,10 @@ class FilterRepositoryTests extends AbstractBaseDataJpaTest {
         when:
         def filter = new ObjectMapper().readValue(entityAttributesFilterJson.bytes, EntityAttributesFilter)
         def persistedFilter = filterRepository.save(filter)
-        entityManager.flush()
-        entityManager.clear()
-
-        then:
         def item1 = filterRepository.findByResourceId("29a5d409-562a-41cd-acee-e9b3d7098d05")
-        entityManager.flush()
-        entityManager.clear()
-        def item2 = filterRepository.findByResourceId("29a5d409-562a-41cd-acee-e9b3d7098d05")
 
-        item1.hashCode() == item2.hashCode()
+        then:
+        item1.hashCode() == persistedFilter.hashCode()
     }
 
     @WithMockAdmin

diff --git a/...y/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepositoryTests.groovy b/...y/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepositoryTests.groovy
@@ -6,8 +6,9 @@ import edu.internet2.tier.shibboleth.admin.ui.security.model.Ownership
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.dao.DataIntegrityViolationException
 import org.springframework.test.annotation.Rollback
+import org.springframework.transaction.annotation.Transactional
 
-import javax.transaction.Transactional
+import javax.persistence.EntityManager
 
 /**
  * Tests to validate the repo and model for groups
@@ -84,7 +85,7 @@ class GroupsRepositoryTests extends AbstractBaseDataJpaTest {
 
         then:
         all.size() == 3
-        savedGroup.ownedItems.size() == 3
+        savedGroup.getOwnedItems().size() == 3
         all.each {
             savedGroup.ownedItems.contains(it)
         }
@@ -133,7 +134,7 @@ class GroupsRepositoryTests extends AbstractBaseDataJpaTest {
         def gList = groupsRepo.findAll()
 
         then:
-        gList.size() == 0
+        gList.isEmpty()
 
         // save check
         when:
@@ -142,6 +143,8 @@ class GroupsRepositoryTests extends AbstractBaseDataJpaTest {
         then:
         // Missing non-nullable field (name) should thrown error
         thrown(DataIntegrityViolationException)
+        entityManager.clear()
+        groupsRepo.findAll().isEmpty()
     }
 
     def "basic CRUD operations validated"() {
@@ -191,7 +194,7 @@ class GroupsRepositoryTests extends AbstractBaseDataJpaTest {
         groupsRepo.delete(groupFromDb2)
 
         then:
-        groupsRepo.findAll().size() == 0
+        groupsRepo.findAll().isEmpty()
 
         when:
         def nothingThere = groupsRepo.findByResourceId(null)