Fixes #656

myconext-server/src/main/java/myconext/cron/InactivityMail.java

@@ -21,6 +21,8 @@
 @Component
 public class InactivityMail {
 
+    public static final long ONE_DAY_IN_MILLIS = 24 * 60 * 60 * 1000L;
+
     private static final Log LOG = LogFactory.getLog(InactivityMail.class);
 
     private final MailBox mailBox;
@@ -50,7 +52,9 @@ public void mailInactiveUsers() {
             return;
         }
         try {
-            Stream.of(UserInactivity.values()).forEach(this::doMailInactiveUsers);
+            //We need to process the users in reverse order to prevent multiple emails to one user in the initial run
+            List<UserInactivity> userInactivities = Stream.of(values()).toList().reversed();
+            userInactivities.forEach(this::doMailInactiveUsers);
             this.doDeleteInactiveUsers();
         } catch (Exception e) {
             //swallow exception as the scheduling stops then
@@ -60,15 +64,15 @@ public void mailInactiveUsers() {
 
     private void doMailInactiveUsers(UserInactivity userInactivity) {
         long nowInMillis = System.currentTimeMillis();
-        long oneDayInMillis = 24 * 60 * 60 * 1000L;
-        long fiveYearsInMillis = 5 * 365 * oneDayInMillis;
+        long fiveYearsInMillis = 5 * 365 * ONE_DAY_IN_MILLIS;
 
-        long lastLoginBefore = nowInMillis - (oneDayInMillis * userInactivity.getInactivityDays());
-        List<User> users = userRepository.findByLastLoginBeforeAndUserInactivity(lastLoginBefore, userInactivity.getPreviousUserInactivity());
+        long lastLoginBefore = nowInMillis - (ONE_DAY_IN_MILLIS * userInactivity.getInactivityDays());
+        List<User> users = userRepository.findByLastLoginBeforeAndUserInactivityIn(lastLoginBefore,
+                this.userInactivitiesWithNullElement(userInactivity.getPreviousUserInactivity()));
 
         Map<String, String> localeVariables = new HashMap<>();
         //This is the future date when the user will be deleted based on the inactivityDays of the userInactivity
-        Date date = new Date(nowInMillis + (fiveYearsInMillis - (oneDayInMillis * userInactivity.getInactivityDays())));
+        Date date = new Date(nowInMillis + (fiveYearsInMillis - (ONE_DAY_IN_MILLIS * userInactivity.getInactivityDays())));
         localeVariables.put("inactivity_period_en", userInactivity.getInactivityPeriodEn());
         localeVariables.put("inactivity_period_nl", userInactivity.getInactivityPeriodNl());
         localeVariables.put("deletion_period_en", userInactivity.getDeletionPeriodEn());
@@ -90,10 +94,18 @@ private void doDeleteInactiveUsers() {
         long oneDayInMillis = 24 * 60 * 60 * 1000L;
 
         long lastLoginBefore = nowInMillis - (oneDayInMillis * 5L * 365);
-        List<User> users = userRepository.findByLastLoginBeforeAndUserInactivity(lastLoginBefore, WEEK_1_BEFORE_5_YEARS);
+        List<User> users = userRepository.findByLastLoginBeforeAndUserInactivityIn(lastLoginBefore, List.of(WEEK_1_BEFORE_5_YEARS));
         userRepository.deleteAll(users);
         LOG.info(String.format("Deleted %s users (%s) who has been inactive for 5 years in for %s ms",
                 users.size(), users.stream().map(User::getEmail).collect(Collectors.joining(", ")),
                 System.currentTimeMillis() - nowInMillis));
     }
+
+    private List<UserInactivity> userInactivitiesWithNullElement(UserInactivity userInactivity) {
+        //Can't use List.of as this does not permit null values
+        List<UserInactivity> userInactivities = new ArrayList<>();
+        userInactivities.add(userInactivity);
+        userInactivities.add(null);
+        return userInactivities;
+    }
 }

myconext-server/src/main/java/myconext/repository/UserRepository.java

@@ -54,7 +54,7 @@ public interface UserRepository extends MongoRepository<User, String> {
     @Query("{'email' : {$regex : ?0, $options: 'i'}}")
     List<User> findByEmailDomain(String regex);
 
-    List<User> findByLastLoginBeforeAndUserInactivity(long lastLoginBefore, UserInactivity userInactivity);
+    List<User> findByLastLoginBeforeAndUserInactivityIn(long lastLoginBefore, List<UserInactivity> userInactivities);
 
     @Query("""
             { $and: [ { $or: [

myconext-server/src/test/java/myconext/cron/InactivityMailTest.java

@@ -17,6 +17,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static myconext.cron.InactivityMail.ONE_DAY_IN_MILLIS;
 import static org.junit.jupiter.api.Assertions.*;
 
 @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
@@ -64,6 +65,30 @@ public void mailInactivityMail() {
         assertThrows(ConditionTimeoutException.class, this::mailMessages);
     }
 
+    @SneakyThrows
+    @Test
+    public void mailInactivityFirstRun() {
+        //See https://github.com/OpenConext/OpenConext-myconext/issues/656
+        User user = new User();
+        long yesterday = System.currentTimeMillis() - ONE_DAY_IN_MILLIS;
+        user.setLastLogin(yesterday - (UserInactivity.WEEK_1_BEFORE_5_YEARS.getInactivityDays() * ONE_DAY_IN_MILLIS));
+        user.setEmail(UserInactivity.WEEK_1_BEFORE_5_YEARS.name());
+        //Explicit set userInactivity to null for self-explanation of the test code
+        user.setUserInactivity(null);
+        userRepository.save(user);
+
+        inactivityMail.mailInactiveUsers();
+
+        List<MimeMessage> mimeMessages = mailMessages();
+        assertEquals(1, mimeMessages.size());
+        User userFromDB = userRepository.findOneUserByEmail(UserInactivity.WEEK_1_BEFORE_5_YEARS.name());
+        assertEquals(UserInactivity.WEEK_1_BEFORE_5_YEARS, userFromDB.getUserInactivity());
+        //Idempotency check
+        greenMail.purgeEmailFromAllMailboxes();
+        inactivityMail.mailInactiveUsers();
+        assertThrows(ConditionTimeoutException.class, this::mailMessages);
+    }
+
     @SneakyThrows
     @Test
     public void mailInactivityMailDutch() {
@@ -88,19 +113,18 @@ private String messageContent(MimeMessage mimeMessage) {
     }
 
     private void inactivityUserSeed(String preferredLanguage) {
-        long oneDayInMillis = 24 * 60 * 60 * 1000L;
-        long yesterday = System.currentTimeMillis() - oneDayInMillis;
+        long yesterday = System.currentTimeMillis() - ONE_DAY_IN_MILLIS;
         Stream.of(UserInactivity.values()).forEach(userInactivity -> {
             User user = new User();
-            user.setLastLogin(yesterday - (userInactivity.getInactivityDays() * oneDayInMillis));
+            user.setLastLogin(yesterday - (userInactivity.getInactivityDays() * ONE_DAY_IN_MILLIS));
             user.setEmail(userInactivity.name());
             user.setPreferredLanguage(preferredLanguage);
             user.setUserInactivity(userInactivity.getPreviousUserInactivity());
             userRepository.save(user);
         });
         //And one extra User who is to be deleted
         User user = new User();
-        user.setLastLogin(yesterday - (((5L * 365) + 5) * oneDayInMillis));
+        user.setLastLogin(yesterday - (((5L * 365) + 5) * ONE_DAY_IN_MILLIS));
         user.setEmail(DELETED_EMAIL);
         user.setUserInactivity(UserInactivity.WEEK_1_BEFORE_5_YEARS);
         userRepository.save(user);