From 039e9a19b3752dbcbc6ce02dac28597dfa5d3bce Mon Sep 17 00:00:00 2001
From: uhensler <urs.hensler@frentix.com>
Date: Mon, 15 Jun 2020 17:37:20 +0200
Subject: [PATCH] OO-4630: Show only really selected appointment in topics list
And some performance optimizations.
---
.../appointments/AppointmentsService.java | 14 +++-
.../appointments/manager/AppointmentDAO.java | 61 ++++++++++++--
.../manager/AppointmentsServiceImpl.java | 11 +++
.../manager/ParticipationDAO.java | 2 +-
.../appointments/ui/TopicsRunController.java | 79 ++++++-------------
.../manager/AppointmentDAOTest.java | 77 ++++++++++++++++++
6 files changed, 180 insertions(+), 64 deletions(-)
diff --git a/src/main/java/org/olat/course/nodes/appointments/AppointmentsService.java b/src/main/java/org/olat/course/nodes/appointments/AppointmentsService.java
index c1073373210..333132c6c40 100644
--- a/src/main/java/org/olat/course/nodes/appointments/AppointmentsService.java
+++ b/src/main/java/org/olat/course/nodes/appointments/AppointmentsService.java
@@ -21,6 +21,7 @@ package org.olat.course.nodes.appointments;
import java.util.Collection;
import java.util.List;
+import java.util.Map;
import org.olat.core.commons.services.notifications.PublisherData;
import org.olat.core.commons.services.notifications.SubscriptionContext;
@@ -65,7 +66,18 @@ public interface AppointmentsService {
public void unconfirmAppointment(Appointment appointment);
public void deleteAppointment(Appointment appointment);
-
+
+ /**
+ * Gets the key of the topic and the according count of appointments.
+ *
+ * @param params
+ * @param freeOnly counts only appointments with free participations
+ * @return the count may be null (instead of 0) if no appointments available.
+ */
+ public Map<Long, Long> getTopicKeyToAppointmentCount(AppointmentSearchParams params, boolean freeOnly);
+
+ public Long getAppointmentCount(AppointmentSearchParams params);
+
public List<Appointment> getAppointments(AppointmentSearchParams params);
public ParticipationResult createParticipation(Appointment appointment, Identity identity,
diff --git a/src/main/java/org/olat/course/nodes/appointments/manager/AppointmentDAO.java b/src/main/java/org/olat/course/nodes/appointments/manager/AppointmentDAO.java
index 94a4f2c0e38..f322a28fd33 100644
--- a/src/main/java/org/olat/course/nodes/appointments/manager/AppointmentDAO.java
+++ b/src/main/java/org/olat/course/nodes/appointments/manager/AppointmentDAO.java
@@ -22,8 +22,13 @@ package org.olat.course.nodes.appointments.manager;
import java.util.Collection;
import java.util.Date;
import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
import javax.persistence.Query;
+import javax.persistence.Tuple;
import javax.persistence.TypedQuery;
import org.olat.core.commons.persistence.DB;
@@ -159,10 +164,58 @@ class AppointmentDAO {
.getResultList();
return appointments.isEmpty() ? null : appointments.get(0);
}
+
+ Map<Long, Long> loadTopicKeyToAppointmentCount(AppointmentSearchParams params, boolean freeOnly) {
+ QueryBuilder sb = new QueryBuilder();
+ sb.append("select appointment.topic.key as topicKey");
+ sb.append(" , appointment.key as key");
+ sb.append(" , appointment.maxParticipations as maxParticipations");
+ sb.append(" , ( select count(*)");
+ sb.append(" from appointmentparticipation participation");
+ sb.append(" where participation.appointment.key = appointment.key");
+ sb.append(" ) as count");
+ appendQuery(sb, params);
+
+ TypedQuery<Tuple> query = dbInstance.getCurrentEntityManager()
+ .createQuery(sb.toString(), Tuple.class);
+ addParameters(query, params);
+
+ List<Tuple> results = query.getResultList();
+ return results.stream()
+ .filter(filterFreeOnly(freeOnly))
+ .map(t -> (Long)t.get(0))
+ .collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
+ }
+
+ private Predicate<? super Tuple> filterFreeOnly(boolean freeOnly) {
+ return t -> freeOnly? t.get(2) == null || ((Integer)t.get(2)).intValue() < ((Long)t.get(3)).intValue(): true;
+ }
+
+ Long loadAppointmentCount(AppointmentSearchParams params) {
+ QueryBuilder sb = new QueryBuilder();
+ sb.append("select count(appointment)");
+ appendQuery(sb, params);
+
+ TypedQuery<Long> query = dbInstance.getCurrentEntityManager()
+ .createQuery(sb.toString(), Long.class);
+ addParameters(query, params);
+
+ return query.getSingleResult();
+ }
List<Appointment> loadAppointments(AppointmentSearchParams params) {
QueryBuilder sb = new QueryBuilder();
sb.append("select appointment");
+ appendQuery(sb, params);
+
+ TypedQuery<Appointment> query = dbInstance.getCurrentEntityManager()
+ .createQuery(sb.toString(), Appointment.class);
+ addParameters(query, params);
+
+ return query.getResultList();
+ }
+
+ private void appendQuery(QueryBuilder sb, AppointmentSearchParams params) {
sb.append(" from appointment appointment");
sb.append(" join").append(" fetch", params.isFetchTopic()).append(" appointment.topic topic");
if (params.getAppointmentKey() != null) {
@@ -183,9 +236,9 @@ class AppointmentDAO {
if (params.getStatus() != null) {
sb.and().append("appointment.status = :status");
}
-
- TypedQuery<Appointment> query = dbInstance.getCurrentEntityManager()
- .createQuery(sb.toString(), Appointment.class);
+ }
+
+ private void addParameters(TypedQuery<?> query, AppointmentSearchParams params) {
if (params.getAppointmentKey() != null) {
query.setParameter("appointmentKey", params.getAppointmentKey());
}
@@ -204,8 +257,6 @@ class AppointmentDAO {
if (params.getStatus() != null) {
query.setParameter("status", params.getStatus());
}
-
- return query.getResultList();
}
}
diff --git a/src/main/java/org/olat/course/nodes/appointments/manager/AppointmentsServiceImpl.java b/src/main/java/org/olat/course/nodes/appointments/manager/AppointmentsServiceImpl.java
index 06469e48a77..1319f2e1f5d 100644
--- a/src/main/java/org/olat/course/nodes/appointments/manager/AppointmentsServiceImpl.java
+++ b/src/main/java/org/olat/course/nodes/appointments/manager/AppointmentsServiceImpl.java
@@ -25,6 +25,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Date;
import java.util.List;
+import java.util.Map;
import java.util.stream.Collectors;
import org.olat.basesecurity.BaseSecurityManager;
@@ -250,6 +251,16 @@ public class AppointmentsServiceImpl implements AppointmentsService {
appointmentsMailing.sendAppointmentDeleted(singletonList(appointment));
appointmentDao.delete(appointment);
}
+
+ @Override
+ public Map<Long, Long> getTopicKeyToAppointmentCount(AppointmentSearchParams params, boolean freeOnly) {
+ return appointmentDao.loadTopicKeyToAppointmentCount(params, freeOnly);
+ }
+
+ @Override
+ public Long getAppointmentCount(AppointmentSearchParams params) {
+ return appointmentDao.loadAppointmentCount(params);
+ }
@Override
public List<Appointment> getAppointments(AppointmentSearchParams params) {
diff --git a/src/main/java/org/olat/course/nodes/appointments/manager/ParticipationDAO.java b/src/main/java/org/olat/course/nodes/appointments/manager/ParticipationDAO.java
index 9d4b83c8e30..c1425b5614f 100644
--- a/src/main/java/org/olat/course/nodes/appointments/manager/ParticipationDAO.java
+++ b/src/main/java/org/olat/course/nodes/appointments/manager/ParticipationDAO.java
@@ -145,7 +145,7 @@ class ParticipationDAO {
return query.getSingleResult();
}
-
+
List<Participation> loadParticipations(ParticipationSearchParams params) {
QueryBuilder sb = new QueryBuilder();
sb.append("select participation");
diff --git a/src/main/java/org/olat/course/nodes/appointments/ui/TopicsRunController.java b/src/main/java/org/olat/course/nodes/appointments/ui/TopicsRunController.java
index ebef86eebdc..d8889a38afc 100644
--- a/src/main/java/org/olat/course/nodes/appointments/ui/TopicsRunController.java
+++ b/src/main/java/org/olat/course/nodes/appointments/ui/TopicsRunController.java
@@ -23,13 +23,11 @@ import static java.util.Collections.emptyList;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
-import java.util.Set;
import java.util.stream.Collectors;
import org.olat.core.gui.UserRequest;
@@ -117,23 +115,11 @@ public class TopicsRunController extends BasicController implements Activateable
}
private void refresh() {
- removeButtons();
+ mainVC.clear();
topics = loadTopicWrappers();
mainVC.contextPut("topics", topics);
}
- private void removeButtons() {
- List<String> componentNames = new ArrayList<>();
- for (Component component : mainVC.getComponents()) {
- if (!"infoSubscription".equals(component.getComponentName())) {
- componentNames.add(component.getComponentName());
- }
- }
- for (String componentName : componentNames) {
- mainVC.remove(componentName);
- }
- }
-
private List<TopicWrapper> loadTopicWrappers() {
List<Topic> topics = appointmentsService.getTopics(entry, subIdent);
Map<Long, List<Organizer>> topicKeyToOrganizer = appointmentsService
@@ -143,37 +129,24 @@ public class TopicsRunController extends BasicController implements Activateable
AppointmentSearchParams aParams = new AppointmentSearchParams();
aParams.setEntry(entry);
aParams.setSubIdent(subIdent);
- Map<Long, List<Appointment>> topicKeyToAppointments = appointmentsService
- .getAppointments(aParams).stream()
- .collect(Collectors.groupingBy(a -> a.getTopic().getKey()));
+ Map<Long, Long> topicKeyToAppointmentCount = appointmentsService.getTopicKeyToAppointmentCount(aParams, true);
ParticipationSearchParams myParticipationsParams = new ParticipationSearchParams();
myParticipationsParams.setEntry(entry);
myParticipationsParams.setSubIdent(subIdent);
myParticipationsParams.setIdentity(getIdentity());
myParticipationsParams.setFetchAppointments(true);
- List<Participation> myParticipations = appointmentsService.getParticipations(myParticipationsParams);
- Map<Long, List<Participation>> topicKeyToMyParticipation = myParticipations.stream()
+ Map<Long, List<Participation>> topicKeyToMyParticipation = appointmentsService
+ .getParticipations(myParticipationsParams).stream()
.collect(Collectors.groupingBy(p -> p.getAppointment().getTopic().getKey()));
- Map<Long, List<Participation>> appointmentsToMyParticipation = myParticipations.stream()
- .collect(Collectors.groupingBy(p -> p.getAppointment().getKey()));
-
- Set<Long> myAppointmentKeys = appointmentsToMyParticipation.keySet();
- ParticipationSearchParams allParticipationParams = new ParticipationSearchParams();
- allParticipationParams.setAppointmentKeys(myAppointmentKeys);
- Map<Long, List<Participation>> appointmentKeyToAllParticipations = appointmentsService
- .getParticipations(allParticipationParams).stream()
- .collect(Collectors.groupingBy(p -> p.getAppointment().getKey()));
List<TopicWrapper> wrappers = new ArrayList<>(topics.size());
for (Topic topic : topics) {
TopicWrapper wrapper = new TopicWrapper(topic);
List<Organizer> organizers = topicKeyToOrganizer.getOrDefault(topic.getKey(), emptyList());
wrapOrganizers(wrapper, organizers);
- List<Appointment> appointments = topicKeyToAppointments.getOrDefault(topic.getKey(), emptyList());
- List<Participation> topicParticipations = topicKeyToMyParticipation.getOrDefault(topic.getKey(), emptyList());
- wrapParticpations(wrapper, topic, topicParticipations, appointments, appointmentsToMyParticipation,
- appointmentKeyToAllParticipations);
+ List<Participation> myTopicParticipations = topicKeyToMyParticipation.getOrDefault(topic.getKey(), emptyList());
+ wrapParticpations(wrapper, topic, myTopicParticipations, topicKeyToAppointmentCount);
wrappers.add(wrapper);
}
return wrappers;
@@ -196,18 +169,19 @@ public class TopicsRunController extends BasicController implements Activateable
}
}
- private void wrapParticpations(TopicWrapper wrapper, Topic topic, List<Participation> participations,
- List<Appointment> appointments, Map<Long, List<Participation>> appointmentKeyToParticipation,
- Map<Long, List<Participation>> appointmentKeyToAllParticipations) {
- if (!participations.isEmpty()) {
+ private void wrapParticpations(TopicWrapper wrapper, Topic topic, List<Participation> myTopicParticipations,
+ Map<Long, Long> topicKeyToAppointmentCount) {
+ if (!myTopicParticipations.isEmpty()) {
Date now = new Date();
- Optional<Appointment> nextAppointment = appointments.stream()
+ Optional<Appointment> nextAppointment = myTopicParticipations.stream()
+ .map(Participation::getAppointment)
.filter(a1 -> now.before(a1.getEnd()))
.sorted((a1, a2) -> a1.getStart().compareTo(a2.getStart()))
.findFirst();
Appointment appointment = nextAppointment.isPresent()
? nextAppointment.get() // Next appointment ...
- : appointments.stream()
+ : myTopicParticipations.stream()
+ .map(Participation::getAppointment)
.sorted((a1, a2) -> a2.getStart().compareTo(a1.getStart()))
.findFirst().get(); // ... or the most recent one.
wrapper.setFuture(Boolean.valueOf(appointment.getStart().after(now)));
@@ -217,16 +191,17 @@ public class TopicsRunController extends BasicController implements Activateable
wrapper.setTranslatedStatus(translate("appointment.status." + appointment.getStatus().name()));
wrapper.setStatusCSS("o_ap_status_" + appointment.getStatus().name());
- List<String> participants = appointmentKeyToAllParticipations
- .getOrDefault(appointment.getKey(), Collections.emptyList()).stream()
+ ParticipationSearchParams allParticipationParams = new ParticipationSearchParams();
+ allParticipationParams.setAppointment(appointment);
+ List<Participation> appointmentParticipations = appointmentsService.getParticipations(allParticipationParams);
+
+ List<String> participants = appointmentParticipations.stream()
.map(p -> userManager.getUserDisplayName(p.getIdentity().getKey()))
.sorted(String.CASE_INSENSITIVE_ORDER)
.collect(Collectors.toList());
wrapper.setParticipants(participants);
- if (participations.size() >= 2) {
- wrapper.setSelectedAppointments(Integer.valueOf(participations.size()));
- }
+ wrapper.setSelectedAppointments(Integer.valueOf(myTopicParticipations.size()));
boolean canChange = config.isMultiParticipations()
? true
@@ -235,12 +210,9 @@ public class TopicsRunController extends BasicController implements Activateable
wrapOpenLink(wrapper, topic, "appointments.change");
}
} else {
- long freeAppointments = appointments.stream()
- .filter(a -> Appointment.Status.planned == a.getStatus())
- .filter(a -> hasFreeParticipations(a, appointmentKeyToParticipation))
- .count();
- wrapper.setFreeAppointments(Long.valueOf(freeAppointments));
- if (freeAppointments > 0) {
+ Long freeAppointments = topicKeyToAppointmentCount.getOrDefault(topic.getKey(), Long.valueOf(0));
+ wrapper.setFreeAppointments(freeAppointments);
+ if (freeAppointments.longValue() > 0) {
wrapOpenLink(wrapper, topic, "appointments.select");
}
}
@@ -301,13 +273,6 @@ public class TopicsRunController extends BasicController implements Activateable
openLink.setUserObject(topic);
wrapper.setOpenLinkName(openLink.getComponentName());
}
-
- private boolean hasFreeParticipations(Appointment appointment, Map<Long, List<Participation>> appointmentKeyToParticipation) {
- if (appointment.getMaxParticipations() == null) return true;
-
- List<Participation> participations = appointmentKeyToParticipation.getOrDefault(appointment.getKey(), emptyList());
- return appointment.getMaxParticipations().intValue() > participations.size();
- }
@Override
protected void event(UserRequest ureq, Controller source, Event event) {
diff --git a/src/test/java/org/olat/course/nodes/appointments/manager/AppointmentDAOTest.java b/src/test/java/org/olat/course/nodes/appointments/manager/AppointmentDAOTest.java
index eb034200fa2..6ce8cbfad43 100644
--- a/src/test/java/org/olat/course/nodes/appointments/manager/AppointmentDAOTest.java
+++ b/src/test/java/org/olat/course/nodes/appointments/manager/AppointmentDAOTest.java
@@ -27,10 +27,12 @@ import java.time.LocalDateTime;
import java.util.Date;
import java.util.GregorianCalendar;
import java.util.List;
+import java.util.Map;
import org.assertj.core.api.SoftAssertions;
import org.junit.Test;
import org.olat.core.commons.persistence.DB;
+import org.olat.core.id.Identity;
import org.olat.core.util.DateUtils;
import org.olat.course.nodes.appointments.Appointment;
import org.olat.course.nodes.appointments.Appointment.Status;
@@ -53,6 +55,8 @@ public class AppointmentDAOTest extends OlatTestCase {
private DB dbInstance;
@Autowired
private TopicDAO topicDao;
+ @Autowired
+ private ParticipationDAO participationDao;
@Autowired
private AppointmentDAO sut;
@@ -239,6 +243,79 @@ public class AppointmentDAOTest extends OlatTestCase {
assertThat(reloadedAppointment).isEqualTo(appointment);
}
+ @Test
+ public void shouldLoadTopicKeyToAppointmentCount() {
+ RepositoryEntry entry = JunitTestHelper.createAndPersistRepositoryEntry();
+ Identity identity = JunitTestHelper.createAndPersistIdentityAsUser(random());
+ String subIdent = JunitTestHelper.random();
+ Topic topic1 = topicDao.createTopic(entry, subIdent);
+ Topic topic2 = topicDao.createTopic(entry, subIdent);
+ Appointment appointment11 = sut.createAppointment(topic1);
+ sut.createAppointment(topic1);
+ Appointment appointment21 = sut.createAppointment(topic2);
+ appointment21.setMaxParticipations(1);
+ sut.saveAppointment(appointment21);
+ participationDao.createParticipation(appointment11, identity);
+ participationDao.createParticipation(appointment11, identity);
+ participationDao.createParticipation(appointment21, identity);
+ dbInstance.commitAndCloseSession();
+
+ AppointmentSearchParams params = new AppointmentSearchParams();
+ params.setEntry(entry);
+ params.setSubIdent(subIdent);
+ Map<Long, Long> appointmentCountByTopic = sut.loadTopicKeyToAppointmentCount(params, false);
+
+ SoftAssertions softly = new SoftAssertions();
+ softly.assertThat(appointmentCountByTopic.get(topic1.getKey())).isEqualTo(2);
+ softly.assertThat(appointmentCountByTopic.get(topic2.getKey())).isEqualTo(1);
+ softly.assertAll();
+ }
+
+ @Test
+ public void shouldLoadTopicKeyToAppointmentCountOnlyFree() {
+ RepositoryEntry entry = JunitTestHelper.createAndPersistRepositoryEntry();
+ Identity identity = JunitTestHelper.createAndPersistIdentityAsUser(random());
+ String subIdent = JunitTestHelper.random();
+ Topic topic1 = topicDao.createTopic(entry, subIdent);
+ Topic topic2 = topicDao.createTopic(entry, subIdent);
+ Appointment appointment11 = sut.createAppointment(topic1);
+ sut.createAppointment(topic1);
+ Appointment appointment21 = sut.createAppointment(topic2);
+ appointment21.setMaxParticipations(1);
+ sut.saveAppointment(appointment21);
+ participationDao.createParticipation(appointment11, identity);
+ participationDao.createParticipation(appointment11, identity);
+ participationDao.createParticipation(appointment21, identity);
+ dbInstance.commitAndCloseSession();
+
+ AppointmentSearchParams params = new AppointmentSearchParams();
+ params.setEntry(entry);
+ params.setSubIdent(subIdent);
+ Map<Long, Long> appointmentCountByTopic = sut.loadTopicKeyToAppointmentCount(params, true);
+
+ SoftAssertions softly = new SoftAssertions();
+ softly.assertThat(appointmentCountByTopic.get(topic1.getKey())).isEqualTo(2);
+ softly.assertThat(appointmentCountByTopic.get(topic2.getKey())).isNull();
+ softly.assertAll();
+ }
+
+ @Test
+ public void shouldLoadCount() {
+ RepositoryEntry entry = JunitTestHelper.createAndPersistRepositoryEntry();
+ Topic topic = topicDao.createTopic(entry, random());
+ Topic topicOther = topicDao.createTopic(entry, random());
+ sut.createAppointment(topic);
+ sut.createAppointment(topic);
+ sut.createAppointment(topicOther);
+ dbInstance.commitAndCloseSession();
+
+ AppointmentSearchParams params = new AppointmentSearchParams();
+ params.setTopic(topic);
+ Long count = sut.loadAppointmentCount(params);
+
+ assertThat(count).isEqualTo(2);
+ }
+
@Test
public void shouldLoadByAppointmentKey() {
RepositoryEntry entry = JunitTestHelper.createAndPersistRepositoryEntry();
--
GitLab