From df1963c24b8296636c914b6a50f9d6c22f0b76df Mon Sep 17 00:00:00 2001 From: zhuoY121 <102482288+zhuoY121@users.noreply.github.com> Date: Sat, 24 Jun 2023 09:51:49 -0400 Subject: [PATCH 1/5] SAK-47604 GB Adding or removing a TA's permissions causes duplicate values --- .../springframework/data/SpringCrudRepositoryImpl.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java index f0872116be69..5f3a0f7408cd 100644 --- a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java +++ b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java @@ -125,7 +125,7 @@ public Iterable findAllById(Iterable ids) { List list = new ArrayList<>(); if (ids != null) { - ids.forEach(id -> findById(id).ifPresent(found -> list.add(found))); + ids.forEach(id -> findById(id).ifPresent(list::add)); } return list; } @@ -137,9 +137,9 @@ public void delete(T entity) { Session session = sessionFactory.getCurrentSession(); try { - session.delete(entity); + findById(entity.getId()).ifPresent(session::delete); } catch (Exception he) { - session.delete(session.merge(entity)); + he.printStackTrace(); } } @@ -161,7 +161,7 @@ public void deleteAll(Iterable entities) { @Override @Transactional public void deleteById(ID id) { - findById(id).ifPresent(found -> delete(found)); + findById(id).ifPresent(this::delete); } /** From 8f0e0c4307c0c57423d14dc0a729572df498656c Mon Sep 17 00:00:00 2001 From: zhuoY121 <102482288+zhuoY121@users.noreply.github.com> Date: Wed, 28 Jun 2023 10:20:56 -0400 Subject: [PATCH 2/5] Add log --- .../springframework/data/SpringCrudRepositoryImpl.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java index 5f3a0f7408cd..33aa68826935 100644 --- a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java +++ b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java @@ -17,6 +17,7 @@ import lombok.Getter; import lombok.Setter; +import lombok.extern.slf4j.Slf4j; import org.hibernate.Criteria; import org.hibernate.Session; import org.hibernate.SessionFactory; @@ -36,6 +37,7 @@ import java.util.List; import java.util.Optional; +@Slf4j @Transactional(readOnly = true) public abstract class SpringCrudRepositoryImpl, ID extends Serializable> implements SpringCrudRepository { @@ -139,7 +141,7 @@ public void delete(T entity) { try { findById(entity.getId()).ifPresent(session::delete); } catch (Exception he) { - he.printStackTrace(); + log.error("Failed to delete the entity: " + he.getMessage()); } } From 8b8b622f3fea95a133ba32f78767e2aa2be03c2e Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Fri, 7 Jul 2023 13:59:45 -0400 Subject: [PATCH 3/5] rename exception var --- .../springframework/data/SpringCrudRepositoryImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java index 33aa68826935..0d851f516dde 100644 --- a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java +++ b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java @@ -140,7 +140,7 @@ public void delete(T entity) { try { findById(entity.getId()).ifPresent(session::delete); - } catch (Exception he) { + } catch (Exception e) { log.error("Failed to delete the entity: " + he.getMessage()); } } From 48faafb4bbde2f6db195acd2670ad5892944d8da Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Fri, 7 Jul 2023 13:59:59 -0400 Subject: [PATCH 4/5] improved logging message --- .../springframework/data/SpringCrudRepositoryImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java index 0d851f516dde..176c37b38d14 100644 --- a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java +++ b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java @@ -141,7 +141,7 @@ public void delete(T entity) { try { findById(entity.getId()).ifPresent(session::delete); } catch (Exception e) { - log.error("Failed to delete the entity: " + he.getMessage()); + log.warn("Could not delete entity [{}:{}], {}", entity.getClass().getName(), entity.getId(), e.toString()); } } From 3f8143ddc3a411cc7d42587a4acebfd667f73946 Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Fri, 7 Jul 2023 16:13:45 -0400 Subject: [PATCH 5/5] deleteById should be used to perform delete --- .../data/SpringCrudRepositoryImpl.java | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java index 176c37b38d14..4d79c4c4a7b0 100644 --- a/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java +++ b/kernel/api/src/main/java/org/sakaiproject/springframework/data/SpringCrudRepositoryImpl.java @@ -29,9 +29,6 @@ import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; -import org.sakaiproject.springframework.data.PersistableEntity; -import org.sakaiproject.springframework.data.Repository; - import java.io.Serializable; import java.util.ArrayList; import java.util.List; @@ -91,14 +88,14 @@ public Iterable saveAll(Iterable entities) { public Optional findById(ID id) { Assert.notNull(id, "The id cannot be null"); - return Optional.ofNullable((T) sessionFactory.getCurrentSession().get(domainClass, id)); + return Optional.ofNullable(sessionFactory.getCurrentSession().get(domainClass, id)); } @Override public T getById(ID id) { Assert.notNull(id, "The id cannot be null"); - return (T) sessionFactory.getCurrentSession().load(domainClass, id); + return sessionFactory.getCurrentSession().load(domainClass, id); } @Override @@ -118,7 +115,7 @@ public Page findAll(Pageable pageable) { Criteria criteria = sessionFactory.getCurrentSession().createCriteria(domainClass); criteria.setFirstResult((int) pageable.getOffset()); - criteria.setMaxResults((int) pageable.getPageSize()); + criteria.setMaxResults(pageable.getPageSize()); return new PageImpl(criteria.list()); } @@ -135,13 +132,10 @@ public Iterable findAllById(Iterable ids) { @Override @Transactional public void delete(T entity) { - - Session session = sessionFactory.getCurrentSession(); - - try { - findById(entity.getId()).ifPresent(session::delete); - } catch (Exception e) { - log.warn("Could not delete entity [{}:{}], {}", entity.getClass().getName(), entity.getId(), e.toString()); + if (entity != null) { + deleteById(entity.getId()); + } else { + log.warn("Can not perform delete on a null entity"); } } @@ -163,7 +157,12 @@ public void deleteAll(Iterable entities) { @Override @Transactional public void deleteById(ID id) { - findById(id).ifPresent(this::delete); + if (id != null) { + Session session = sessionFactory.getCurrentSession(); + findById(id).ifPresent(session::delete); + } else { + log.warn("Can not perform delete with a null id"); + } } /**