Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAK-49949 tasks Soft delete tasks for soft deleted sites #12945

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

SAK-49949 tasks Soft delete tasks for soft deleted sites #12945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions kernel/api/src/main/java/org/sakaiproject/site/api/SiteService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ public interface SiteService extends EntityProducer

/** Name for the event for soft deleting a site */
static final String SOFT_DELETE_SITE = "site.del.soft";

/** Name for the event for restoring a soft deleted site */
static final String SITE_RESTORED = "site.restored";

/** Name for the event of removing a site that has already been softly deleted */
static final String SECURE_REMOVE_SOFTLY_DELETED_SITE = "site.del.softly.deleted";
Expand Down
4 changes: 3 additions & 1 deletion kernel/api/src/main/java/org/sakaiproject/tasks/api/Task.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,7 @@ public class Task implements PersistableEntity<Long> {

@Column(name = "TASK_OWNER", length = 99)
private String owner;


@Column(name = "SOFT_DELETED")
adrianfish marked this conversation as resolved.
Show resolved Hide resolved
private Boolean softDeleted;
}
2 changes: 2 additions & 0 deletions kernel/api/src/main/java/org/sakaiproject/tasks/api/repository/TaskRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

public interface TaskRepository extends SpringCrudRepository<Task, Long> {

List<Task> findBySiteId(String siteId);
Optional<Task> findByReference(String reference);
List<Task> findByGroupsContaining(String groupId);
int setSoftDeletedBySiteId(String siteId, Boolean softDeleted);
}
7 changes: 4 additions & 3 deletions kernel/api/src/main/java/org/sakaiproject/tasks/api/repository/UserTaskRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@

public interface UserTaskRepository extends SpringCrudRepository<UserTask, Long> {

List<UserTask> findBySiteId(String siteId);
List<UserTask> findByTaskIdAndUserIdIn(Long taskId, List<String> userIds);
List<UserTask> findByUserIdAndStartsAfter(String userId, Instant from);
List<UserTask> findByUserIdAndStartsAfterAndSoftDeleted(String userId, Instant from, Boolean softDeleted);
List<UserTask> findByUserIdAndSiteId(String userId, String siteId);
List<UserTask> findByUserIdAndTask_StartsLessThanEqual(String userId, Instant instant);
List<UserTask> findByTask_SiteId(String siteId);
void deleteByTask(Task task);
void deleteByTaskAndUserIdNotIn(Task task, Set<String> users);
int deleteByTask(Task task);
int deleteByTaskAndUserIdNotIn(Task task, Set<String> users);
}
67 changes: 58 additions & 9 deletions kernel/kernel-impl/src/main/java/org/sakaiproject/tasks/impl/TaskServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@

import org.springframework.beans.BeanUtils;
import org.springframework.beans.factory.annotation.Autowired;

import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionTemplate;

Expand All @@ -72,15 +73,17 @@ public class TaskServiceImpl implements TaskService, Observer {
@Autowired private EntityManager entityManager;
@Autowired private EventTrackingService eventTrackingService;
@Autowired private FunctionManager functionManager;

@Qualifier("org.sakaiproject.springframework.orm.hibernate.GlobalTransactionManager")
@Autowired private PlatformTransactionManager transactionManager;

@Autowired private SecurityService securityService;
@Autowired private SessionManager sessionManager;
@Autowired private SiteService siteService;
@Autowired private TaskAssignedRepository taskAssignedRepository;
@Autowired private TaskRepository taskRepository;
@Autowired private UserTaskRepository userTaskRepository;

@Setter private TransactionTemplate transactionTemplate;

public void init() {

functionManager.registerFunction(TaskPermissions.CREATE_TASK, true);
Expand All @@ -95,7 +98,10 @@ public void update(Observable o, Object arg) {
if (event.getEvent().equals(SiteService.SECURE_UPDATE_SITE_MEMBERSHIP)) {
try {
Set<String> siteUsers = siteService.getSite(event.getContext()).getUsers();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is out of scope for your PR, just an FYI for anyone following. event.getContext() is usually null here. also it can get called thousands of times in quick succession during SIS updates ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically, if students are being added one by one, this will be a big hit as it will be pulling the list of students from the site on every event?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it's just an annoyance as this can be a big NPE .... i'm okay with the getUsers pull because it should be cached

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it get cached though, if the site members have been updated before the event was fired? Anyway, how often would a big SIS update like that happen?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe five times a day

transactionTemplate.executeWithoutResult(status -> {

TransactionTemplate tt = new TransactionTemplate(transactionManager);

tt.executeWithoutResult(status -> {

userTaskRepository.findByTask_SiteId(event.getContext()).forEach(userTask -> {

Expand All @@ -112,7 +118,10 @@ public void update(Observable o, Object arg) {
String groupId = event.getResource();
try {
String groupRef = AuthzGroupReferenceBuilder.builder().site(event.getContext()).group(groupId).build();
transactionTemplate.executeWithoutResult(status -> {

TransactionTemplate tt = new TransactionTemplate(transactionManager);

tt.executeWithoutResult(status -> {

// Find any task containing this group
taskRepository.findByGroupsContaining(groupRef).forEach(t -> {
Expand All @@ -134,6 +143,46 @@ public void update(Observable o, Object arg) {
} catch (Exception e) {
log.error("Failed to update user tasks for group {}: {}", groupId, e.toString());
}
} else if (event.getEvent().equals(SiteService.SOFT_DELETE_SITE)) {

String siteId = event.getContext();

TransactionTemplate tt = new TransactionTemplate(transactionManager);

tt.executeWithoutResult(status -> {

userTaskRepository.findBySiteId(siteId).forEach(ut -> {

ut.setSoftDeleted(Boolean.TRUE);
userTaskRepository.save(ut);
});

taskRepository.findBySiteId(siteId).forEach(t -> {

t.setSoftDeleted(Boolean.TRUE);
taskRepository.save(t);
});
});
} else if (event.getEvent().equals(SiteService.SITE_RESTORED)) {

String siteId = event.getContext();

TransactionTemplate tt = new TransactionTemplate(transactionManager);

tt.executeWithoutResult(status -> {

userTaskRepository.findBySiteId(siteId).forEach(ut -> {

ut.setSoftDeleted(Boolean.FALSE);
userTaskRepository.save(ut);
});

taskRepository.findBySiteId(siteId).forEach(t -> {

t.setSoftDeleted(Boolean.FALSE);
taskRepository.save(t);
});
});
}
}
}
Expand Down Expand Up @@ -162,9 +211,7 @@ public Task createTask(Task task, Set<String> users, Integer priority) {

Optional<Task> optionalCurrentTask = getTask(task.getReference());

if (optionalCurrentTask.isPresent()) {
task.setId(optionalCurrentTask.get().getId());
}
optionalCurrentTask.ifPresent(t -> task.setId(t.getId()));

final Task mergedTask = taskRepository.save(task);

Expand Down Expand Up @@ -217,6 +264,7 @@ public UserTask saveUserTask(UserTaskAdapterBean transfer) {

@Transactional
public UserTask createUserTask(Task task, UserTaskAdapterBean transfer) {

UserTask userTask = new UserTask();
userTask.setTask(task);
BeanUtils.copyProperties(transfer, userTask);
Expand All @@ -241,9 +289,10 @@ public List<UserTaskAdapterBean> getCurrentTasksForCurrentUser() {

String userId = sessionManager.getCurrentSessionUserId();

return userTaskRepository.findByUserIdAndStartsAfter(userId, Instant.now())
return userTaskRepository.findByUserIdAndStartsAfterAndSoftDeleted(userId, Instant.now(), Boolean.FALSE)
.stream()
.map(ut -> {

UserTaskAdapterBean bean = new UserTaskAdapterBean();
BeanUtils.copyProperties(ut, bean);
BeanUtils.copyProperties(ut.getTask(), bean);
Expand Down
25 changes: 25 additions & 0 deletions .../kernel-impl/src/main/java/org/sakaiproject/tasks/impl/repository/TaskRepositoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.CriteriaUpdate;
import javax.persistence.criteria.Root;

import org.hibernate.Session;
Expand All @@ -36,6 +37,18 @@

public class TaskRepositoryImpl extends SpringCrudRepositoryImpl<Task, Long> implements TaskRepository {

public List<Task> findBySiteId(String siteId) {

Session session = sessionFactory.getCurrentSession();

CriteriaBuilder cb = session.getCriteriaBuilder();
CriteriaQuery<Task> cq = cb.createQuery(Task.class);
Root<Task> root = cq.from(Task.class);
cq.where(cb.equal(root.get("siteId"), siteId));

return session.createQuery(cq).list();
}

public Optional<Task> findByReference(String reference) {

Session session = sessionFactory.getCurrentSession();
Expand All @@ -60,4 +73,16 @@ public List<Task> findByGroupsContaining(String groupId) {

return session.createQuery(cq).list();
}

public int setSoftDeletedBySiteId(String siteId, Boolean softDeleted) {

Session session = sessionFactory.getCurrentSession();

CriteriaBuilder cb = session.getCriteriaBuilder();
CriteriaUpdate<Task> cu = cb.createCriteriaUpdate(Task.class);
Root<Task> root = cu.from(Task.class);
cu.set("softDeleted", softDeleted).where(cb.equal(root.get("siteId"), siteId));

return session.createQuery(cu).executeUpdate();
}
}
63 changes: 43 additions & 20 deletions ...nel-impl/src/main/java/org/sakaiproject/tasks/impl/repository/UserTaskRepositoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import java.time.Instant;

import org.hibernate.Session;
import org.hibernate.criterion.Restrictions;

import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaDelete;
Expand All @@ -41,23 +40,39 @@

public class UserTaskRepositoryImpl extends SpringCrudRepositoryImpl<UserTask, Long> implements UserTaskRepository {

public List<UserTask> findBySiteId(String siteId) {

Session session = sessionFactory.getCurrentSession();

CriteriaBuilder cb = session.getCriteriaBuilder();
CriteriaQuery<UserTask> query = cb.createQuery(UserTask.class);
Root<UserTask> userTask = query.from(UserTask.class);
query.where(cb.equal(userTask.get("task").get("siteId"), siteId));

return session.createQuery(query).list();
}

public List<UserTask> findByTaskIdAndUserIdIn(Long taskId, List<String> userIds) {

Session session = sessionFactory.getCurrentSession();

return (List<UserTask>) session.createCriteria(UserTask.class)
.add(Restrictions.eq("task.id", taskId))
.add(Restrictions.in("userId", userIds)).list();
CriteriaBuilder cb = session.getCriteriaBuilder();
CriteriaQuery<UserTask> query = cb.createQuery(UserTask.class);
Root<UserTask> userTask = query.from(UserTask.class);
query.where(cb.equal(userTask.get("task").get("id"), taskId), userTask.get("userId").in(userIds));

return session.createQuery(query).list();
}

public List<UserTask> findByUserIdAndStartsAfter(String userId, Instant from) {
public List<UserTask> findByUserIdAndStartsAfterAndSoftDeleted(String userId, Instant from, Boolean softDeleted) {

Session session = sessionFactory.getCurrentSession();

CriteriaBuilder cb = session.getCriteriaBuilder();
CriteriaQuery<UserTask> query = cb.createQuery(UserTask.class);
Root<UserTask> userTask = query.from(UserTask.class);
query.where(cb.equal(userTask.get("userId"), userId), cb.lessThanOrEqualTo(userTask.get("task").get("starts"), from));
query.where(cb.and(cb.equal(userTask.get("userId"), userId) , cb.lessThanOrEqualTo(userTask.get("task").get("starts"), from))
, cb.or(cb.isNull(userTask.get("softDeleted")), cb.equal(userTask.get("softDeleted"), softDeleted)));

return session.createQuery(query).list();
}
Expand All @@ -66,18 +81,24 @@ public List<UserTask> findByUserIdAndSiteId(String userId, String siteId) {

Session session = sessionFactory.getCurrentSession();

return session.createQuery("select u from UserTask u where userId = :userId and task.siteId = :siteId")
.setParameter("userId", userId)
.setParameter("siteId", siteId).list();
CriteriaBuilder cb = session.getCriteriaBuilder();
CriteriaQuery<UserTask> query = cb.createQuery(UserTask.class);
Root<UserTask> userTask = query.from(UserTask.class);
query.where(cb.equal(userTask.get("userId"), userId), cb.equal(userTask.get("task").get("siteId"), siteId));

return session.createQuery(query).list();
}

public List<UserTask> findByUserIdAndTask_StartsLessThanEqual(String userId, Instant instant) {
public List<UserTask> findByUserIdAndTask_StartsLessThanEqual(String userId, Instant earlierThan) {

Session session = sessionFactory.getCurrentSession();

return (List<UserTask>) session.createCriteria(UserTask.class)
.add(Restrictions.eq("userId", userId))
.add(Restrictions.le("task.starts", instant)).list();
CriteriaBuilder cb = session.getCriteriaBuilder();
CriteriaQuery<UserTask> query = cb.createQuery(UserTask.class);
Root<UserTask> userTask = query.from(UserTask.class);
query.where(cb.equal(userTask.get("userId"), userId), cb.lessThanOrEqualTo(userTask.get("task").get("starts"), earlierThan));

return session.createQuery(query).list();
}

public List<UserTask> findByTask_SiteId(String siteId) {
Expand All @@ -92,19 +113,21 @@ public List<UserTask> findByTask_SiteId(String siteId) {
cq.where(cb.equal(taskJoin.get("siteId"), siteId));

return session.createQuery(cq).list();

}

public void deleteByTask(Task task) {
public int deleteByTask(Task task) {

Session session = sessionFactory.getCurrentSession();

session.createQuery("delete from UserTask where task = :task")
.setParameter("task", task).executeUpdate();
session.delete(task);
CriteriaBuilder cb = session.getCriteriaBuilder();
CriteriaDelete<UserTask> cd = cb.createCriteriaDelete(UserTask.class);
Root<UserTask> userTask = cd.from(UserTask.class);
cd.where(cb.equal(userTask.get("task"), task));

return session.createQuery(cd).executeUpdate();
}

public void deleteByTaskAndUserIdNotIn(Task task, Set<String> users) {
public int deleteByTaskAndUserIdNotIn(Task task, Set<String> users) {

Session session = sessionFactory.getCurrentSession();

Expand All @@ -113,6 +136,6 @@ public void deleteByTaskAndUserIdNotIn(Task task, Set<String> users) {
Root<UserTask> root = cd.from(UserTask.class);
cd.where(cb.equal(root.get("task"), task), cb.not(root.get("userId").in(users)));

session.createQuery(cd).executeUpdate();
return session.createQuery(cd).executeUpdate();
}
}
6 changes: 0 additions & 6 deletions kernel/kernel-impl/src/main/webapp/WEB-INF/tasks-components.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@

<bean id="org.sakaiproject.tasks.api.TaskService"
class="org.sakaiproject.tasks.impl.TaskServiceImpl" init-method="init">

<property name="transactionTemplate">
<bean class="org.springframework.transaction.support.TransactionTemplate">
<property name="transactionManager" ref="org.sakaiproject.springframework.orm.hibernate.GlobalTransactionManager"/>
</bean>
</property>
</bean>

<bean id="org.sakaiproject.springframework.orm.hibernate.impl.AdditionalHibernateMappings.taskservice"
Expand Down
Loading
Loading