fix: (2.39) notification trigger cause server halt [DHIS2-19452] (#20576)

Author: zubaira

dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/notification/template/snapshot/NotificationTemplateMapper.java

@@ -27,19 +27,16 @@
  */
 package org.hisp.dhis.program.notification.template.snapshot;
 
-import static java.util.Collections.singleton;
-
-import com.google.common.collect.ImmutableList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 import org.hisp.dhis.common.BaseIdentifiableObject;
 import org.hisp.dhis.dataelement.DataElement;
-import org.hisp.dhis.organisationunit.OrganisationUnit;
 import org.hisp.dhis.program.notification.ProgramNotificationTemplate;
 import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
 import org.hisp.dhis.user.User;
@@ -54,7 +51,7 @@ public ProgramNotificationTemplate toProgramNotificationTemplate(
     return toBaseIdentifiableObject(
         templateSnapshot,
         ProgramNotificationTemplate::new,
-        ImmutableList.of(
+        List.of(
             t -> t.setMessageTemplate(templateSnapshot.getMessageTemplate()),
             t -> t.setNotificationRecipient(templateSnapshot.getNotificationRecipient()),
             t ->
@@ -85,7 +82,7 @@ public ProgramNotificationTemplateSnapshot toProgramNotificationTemplateSnapshot
     return toIdentifiableObjectSnapshot(
         template,
         ProgramNotificationTemplateSnapshot::new,
-        ImmutableList.of(
+        List.of(
             t -> t.setMessageTemplate(template.getMessageTemplate()),
             t -> t.setNotificationRecipient(template.getNotificationRecipient()),
             t ->
@@ -115,14 +112,14 @@ private UserGroup toUserGroup(UserGroupSnapshot userGroupSnapshot) {
     return toBaseIdentifiableObject(
         userGroupSnapshot,
         UserGroup::new,
-        ImmutableList.of(ug -> ug.setMembers(toUsers(userGroupSnapshot.getMembers()))));
+        List.of(ug -> ug.setMembers(toUsers(userGroupSnapshot.getMembers()))));
   }
 
   private UserGroupSnapshot toUserGroupSnapshot(UserGroup userGroup) {
     return toIdentifiableObjectSnapshot(
         userGroup,
         UserGroupSnapshot::new,
-        ImmutableList.of(ug -> ug.setMembers(toUserSnapshot(userGroup.getMembers(), 0))));
+        List.of(ug -> ug.setMembers(toUserSnapshot(userGroup.getMembers()))));
   }
 
   private Set<User> toUsers(Set<UserSnapshot> userSnapshots) {
@@ -133,74 +130,32 @@ private Set<User> toUsers(Set<UserSnapshot> userSnapshots) {
           toBaseIdentifiableObject(
               userSnapshot,
               User::new,
-              ImmutableList.of(
+              List.of(
                   u -> u.setName(userSnapshot.getName()),
                   u -> u.setUsername(userSnapshot.getUsername()),
                   u -> u.setEmail(userSnapshot.getEmail()),
-                  u -> u.setPhoneNumber(userSnapshot.getPhoneNumber()),
-                  u ->
-                      u.setOrganisationUnits(
-                          singleton(toOrganisationUnit(userSnapshot.getOrganisationUnit()))))));
+                  u -> u.setPhoneNumber(userSnapshot.getPhoneNumber()))));
     }
     return users;
   }
 
-  private Set<UserSnapshot> toUserSnapshot(Set<User> users, int ouLevel) {
+  private Set<UserSnapshot> toUserSnapshot(Set<User> users) {
     Set<UserSnapshot> userSnapshots = new HashSet<>();
 
     for (User user : users) {
       userSnapshots.add(
           toIdentifiableObjectSnapshot(
               user,
               UserSnapshot::new,
-              ImmutableList.of(
+              List.of(
                   u -> u.setName(user.getName()),
                   u -> u.setUsername(user.getUsername()),
                   u -> u.setEmail(user.getEmail()),
-                  u -> u.setPhoneNumber(user.getPhoneNumber()),
-                  u ->
-                      u.setOrganisationUnit(
-                          toOrganisationUnitSnapshot(user.getOrganisationUnit(), ouLevel)))));
+                  u -> u.setPhoneNumber(user.getPhoneNumber()))));
     }
     return userSnapshots;
   }
 
-  private OrganisationUnit toOrganisationUnit(OrganisationUnitSnapshot organisationUnitSnapshot) {
-    return toBaseIdentifiableObject(
-        organisationUnitSnapshot,
-        OrganisationUnit::new,
-        ImmutableList.of(
-            ou -> ou.setName(organisationUnitSnapshot.getName()),
-            ou -> ou.setDescription(organisationUnitSnapshot.getDescription()),
-            ou -> ou.setShortName(organisationUnitSnapshot.getShortName()),
-            ou ->
-                ou.setParent(
-                    organisationUnitSnapshot.getParent() != null
-                        ? toOrganisationUnit(organisationUnitSnapshot.getParent())
-                        : null),
-            ou -> ou.setUsers(toUsers(organisationUnitSnapshot.getUsers()))));
-  }
-
-  private OrganisationUnitSnapshot toOrganisationUnitSnapshot(
-      OrganisationUnit organisationUnit, int level) {
-    if (level < 2) {
-      return toIdentifiableObjectSnapshot(
-          organisationUnit,
-          OrganisationUnitSnapshot::new,
-          ImmutableList.of(
-              ou -> ou.setName(organisationUnit.getName()),
-              ou -> ou.setDescription(organisationUnit.getDescription()),
-              ou -> ou.setShortName(organisationUnit.getShortName()),
-              ou ->
-                  ou.setParent(
-                      organisationUnit.getParent() != null
-                          ? toOrganisationUnitSnapshot(organisationUnit.getParent(), level + 1)
-                          : null),
-              ou -> ou.setUsers(toUserSnapshot(organisationUnit.getUsers(), level + 1))));
-    }
-    return null;
-  }
-
   private <T extends IdentifiableObjectSnapshot> T toIdentifiableObjectSnapshot(
       BaseIdentifiableObject from,
       Supplier<T> instanceSupplier,

dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/notification/template/snapshot/OrganisationUnitSnapshot.java

@@ -40,6 +40,4 @@ public class UserSnapshot extends IdentifiableObjectSnapshot {
   private String email;
 
   private String phoneNumber;
-
-  private OrganisationUnitSnapshot organisationUnit;
 }

dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/notification/template/snapshot/UserSnapshot.java

@@ -75,7 +75,6 @@
 import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue;
 import org.hisp.dhis.user.User;
 import org.hisp.dhis.user.UserGroup;
-import org.hisp.dhis.util.DateUtils;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
@@ -89,9 +88,7 @@ public class DefaultProgramNotificationService implements ProgramNotificationSer
   private static final Predicate<NotificationInstanceWithTemplate> IS_SCHEDULED_BY_PROGRAM_RULE =
       (iwt) ->
           Objects.nonNull(iwt.getProgramNotificationInstance())
-              && PROGRAM_RULE.equals(iwt.getProgramNotificationTemplate().getNotificationTrigger())
-              && iwt.getProgramNotificationInstance().getScheduledAt() != null
-              && DateUtils.isToday(iwt.getProgramNotificationInstance().getScheduledAt());
+              && PROGRAM_RULE.equals(iwt.getProgramNotificationTemplate().getNotificationTrigger());
 
   // -------------------------------------------------------------------------
   // Dependencies
@@ -107,6 +104,8 @@ public class DefaultProgramNotificationService implements ProgramNotificationSer
 
   @NonNull private final IdentifiableObjectManager identifiableObjectManager;
 
+  @NonNull private final ProgramNotificationInstanceService notificationInstanceService;
+
   @NonNull private final NotificationMessageRenderer<ProgramInstance> programNotificationRenderer;
 
   @NonNull
@@ -148,11 +147,15 @@ public void sendScheduledNotificationsForDay(Date notificationDate, JobProgress
   public void sendScheduledNotifications(JobProgress progress) {
     progress.startingStage(
         "Fetching and filtering ProgramStageNotification messages scheduled by program rules");
+
+    ProgramNotificationInstanceParam param =
+        ProgramNotificationInstanceParam.builder().scheduledAt(new Date()).build();
+
     List<NotificationInstanceWithTemplate> instancesWithTemplates =
         progress.runStage(
             List.of(),
             () ->
-                identifiableObjectManager.getAll(ProgramNotificationInstance.class).stream()
+                notificationInstanceService.getProgramNotificationInstances(param).stream()
                     .map(this::withTemplate)
                     .filter(this::hasTemplate)
                     .filter(IS_SCHEDULED_BY_PROGRAM_RULE)

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/program/notification/DefaultProgramNotificationService.java

@@ -27,12 +27,18 @@
  */
 package org.hisp.dhis.program.notification;
 
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
 import java.util.ArrayList;
+import java.util.Date;
 import java.util.List;
 import java.util.function.Function;
 import javax.persistence.criteria.CriteriaBuilder;
 import javax.persistence.criteria.Predicate;
 import javax.persistence.criteria.Root;
+import org.apache.commons.lang3.tuple.Pair;
 import org.hibernate.SessionFactory;
 import org.hisp.dhis.common.hibernate.HibernateIdentifiableObjectStore;
 import org.hisp.dhis.hibernate.JpaQueryParameters;
@@ -118,9 +124,30 @@ private List<Function<Root<ProgramNotificationInstance>, Predicate>> getPredicat
     }
 
     if (params.hasScheduledAt()) {
-      predicates.add(root -> builder.equal(root.get("scheduledAt"), params.getScheduledAt()));
+      Pair<Date, Date> scheduledAt = getStartAndEndOfDay(params.getScheduledAt());
+
+      predicates.add(
+          root ->
+              builder.and(
+                  builder.greaterThanOrEqualTo(root.get("scheduledAt"), scheduledAt.getLeft()),
+                  builder.lessThan(root.get("scheduledAt"), scheduledAt.getRight())));
     }
 
     return predicates;
   }
+
+  public Pair<Date, Date> getStartAndEndOfDay(Date scheduledAt) {
+    ZoneId zoneId = ZoneId.systemDefault();
+    Instant instant = scheduledAt.toInstant();
+    LocalDate localDate = instant.atZone(zoneId).toLocalDate();
+
+    ZonedDateTime startOfDay = localDate.atStartOfDay(zoneId);
+
+    ZonedDateTime startOfNextDay = localDate.plusDays(1).atStartOfDay(zoneId);
+
+    Date start = Date.from(startOfDay.toInstant());
+    Date end = Date.from(startOfNextDay.toInstant());
+
+    return Pair.of(start, end);
+  }
 }

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/program/notification/HibernateProgramNotificationInstanceStore.java

@@ -120,6 +120,8 @@ class ProgramNotificationServiceTest extends DhisConvenienceTest {
 
   @Mock private ProgramNotificationTemplateService notificationTemplateService;
 
+  @Mock private ProgramNotificationInstanceService notificationInstanceService;
+
   private NotificationTemplateMapper notificationTemplateMapper = new NotificationTemplateMapper();
 
   private DefaultProgramNotificationService programNotificationService;
@@ -197,6 +199,7 @@ public void initTest() {
             this.programInstanceStore,
             this.programStageInstanceStore,
             this.manager,
+            this.notificationInstanceService,
             this.programNotificationRenderer,
             this.programStageNotificationRenderer,
             notificationTemplateService,
@@ -658,13 +661,13 @@ void testScheduledNotifications() {
               return new BatchResponseStatus(Collections.emptyList());
             });
 
-    when(manager.getAll(ProgramNotificationInstance.class))
-        .thenReturn(Collections.singletonList(programNotificationInstaceForToday));
-
     when(programNotificationRenderer.render(
             any(ProgramInstance.class), any(NotificationTemplate.class)))
         .thenReturn(notificationMessage);
 
+    when(notificationInstanceService.getProgramNotificationInstances(any()))
+        .thenReturn(List.of(programNotificationInstaceForToday));
+
     programNotificationService.sendScheduledNotifications(NoopJobProgress.INSTANCE);
 
     assertEquals(1, sentProgramMessages.size());