diff --git a/client/pom.xml b/client/pom.xml index 4d1737a2..68b0db98 100644 --- a/client/pom.xml +++ b/client/pom.xml @@ -4,7 +4,7 @@ org.openconext invite - 1.1.1-SNAPSHOT + 1.1.2-SNAPSHOT ../pom.xml invite-client diff --git a/pom.xml b/pom.xml index 1ca444b3..e056f968 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.openconext invite - 1.1.1-SNAPSHOT + 1.1.2-SNAPSHOT pom invite SURFconext Invite diff --git a/provisioning-mock/pom.xml b/provisioning-mock/pom.xml index 811df825..89739f52 100644 --- a/provisioning-mock/pom.xml +++ b/provisioning-mock/pom.xml @@ -4,7 +4,7 @@ org.openconext invite - 1.1.1-SNAPSHOT + 1.1.2-SNAPSHOT ../pom.xml provisioning-mock diff --git a/server/pom.xml b/server/pom.xml index af8c8145..c8b07012 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -4,7 +4,7 @@ org.openconext invite - 1.1.1-SNAPSHOT + 1.1.2-SNAPSHOT ../pom.xml invite-server @@ -87,6 +87,16 @@ + + net.javacrumbs.shedlock + shedlock-spring + 7.7.0 + + + net.javacrumbs.shedlock + shedlock-provider-jdbc-template + 7.7.0 + org.springframework.security spring-security-core diff --git a/server/src/main/java/invite/cron/AbstractNodeLeader.java b/server/src/main/java/invite/cron/AbstractNodeLeader.java deleted file mode 100644 index a2b3380b..00000000 --- a/server/src/main/java/invite/cron/AbstractNodeLeader.java +++ /dev/null @@ -1,102 +0,0 @@ -package invite.cron; - -import lombok.SneakyThrows; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - -import javax.sql.DataSource; -import java.sql.Connection; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; - -public abstract class AbstractNodeLeader { - - private static final Log LOG = LogFactory.getLog(AbstractNodeLeader.class); - - private final String lockName; - private final DataSource dataSource; - - protected AbstractNodeLeader(String lockName, DataSource dataSource) { - this.lockName = lockName; - this.dataSource = dataSource; - } - - @SneakyThrows - public void perform(String name, Executable executable) { - Connection conn = null; - boolean lockAcquired = false; - - try { - conn = dataSource.getConnection(); - lockAcquired = tryGetLock(conn, lockName); - - if (!lockAcquired) { - LOG.info(String.format("Another node is running %s, skipping this one", name)); - //Might be that there is a lock not cleaned up due to VM crash - this.cleanupStaleLocks(conn, 60); - return; - } - - LOG.info(String.format("Lock acquired for %s", name)); - executable.execute(); - LOG.info(String.format("Executable %s completed successfully", name)); - } catch (Throwable e) { - LOG.error(String.format("Error occurred in %s", name), e); - } finally { - if (lockAcquired) { - try { - releaseLock(conn, lockName); - LOG.info(String.format("Lock released for %s", name)); - } catch (Exception e) { - LOG.error(String.format("Failed to release lock %s", name), e); - } - } - - if (conn != null) { - try { - conn.close(); - } catch (Exception ignored) { - //Can't do anything about this - LOG.warn(String.format("Failed to close lock %s", name)); - } - } - } - } - - protected boolean tryGetLock(Connection conn, String name) throws Exception { - try { - conn.setAutoCommit(false); - try (PreparedStatement ps = conn.prepareStatement( - "INSERT INTO distributed_locks (lock_name, acquired_at) VALUES (?, NOW())")) { - ps.setString(1, name); - ps.executeUpdate(); - conn.commit(); - return true; - } catch (SQLException e) { - conn.rollback(); - // Duplicate key or other constraint violation means lock is held - return false; - } - } finally { - conn.setAutoCommit(true); - } - } - - private void releaseLock(Connection conn, String name) throws Exception { - try (PreparedStatement ps = conn.prepareStatement( - "DELETE FROM distributed_locks WHERE lock_name = ?")) { - ps.setString(1, name); - ps.executeUpdate(); - } - } - - private void cleanupStaleLocks(Connection conn, int timeoutMinutes) throws Exception { - try (PreparedStatement ps = conn.prepareStatement( - "DELETE FROM distributed_locks WHERE acquired_at < NOW() - INTERVAL ? MINUTE")) { - ps.setInt(1, timeoutMinutes); - ps.executeUpdate(); - } - } - -} diff --git a/server/src/main/java/invite/cron/Executable.java b/server/src/main/java/invite/cron/Executable.java deleted file mode 100644 index 22d112ef..00000000 --- a/server/src/main/java/invite/cron/Executable.java +++ /dev/null @@ -1,8 +0,0 @@ -package invite.cron; - -@FunctionalInterface -public interface Executable { - - void execute() throws Throwable; - -} diff --git a/server/src/main/java/invite/cron/ResourceCleaner.java b/server/src/main/java/invite/cron/ResourceCleaner.java index af86c396..273c7ec2 100644 --- a/server/src/main/java/invite/cron/ResourceCleaner.java +++ b/server/src/main/java/invite/cron/ResourceCleaner.java @@ -3,7 +3,6 @@ import invite.audit.UserRoleAuditService; import invite.model.Role; -import invite.model.Status; import invite.model.User; import invite.model.UserRole; import invite.model.UserRoleAudit; @@ -12,6 +11,7 @@ import invite.repository.UserRepository; import invite.repository.UserRoleAuditRepository; import invite.repository.UserRoleRepository; +import net.javacrumbs.shedlock.spring.annotation.SchedulerLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -20,14 +20,13 @@ import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; -import javax.sql.DataSource; import java.time.Instant; import java.time.Period; import java.util.List; import java.util.Map; @Component -public class ResourceCleaner extends AbstractNodeLeader { +public class ResourceCleaner { public static final String LOCK_NAME = "resource_cleaner_user_level_lock"; private static final Log LOG = LogFactory.getLog(ResourceCleaner.class); @@ -47,14 +46,12 @@ public class ResourceCleaner extends AbstractNodeLeader { public ResourceCleaner(UserRepository userRepository, UserRoleRepository userRoleRepository, ProvisioningService provisioningService, - DataSource dataSource, UserRoleAuditRepository userRoleAuditRepository, UserRoleAuditService userRoleAuditService, InvitationRepository invitationRepository, @Value("${cron.last-activity-duration-days}") int lastActivityDurationDays, @Value("${cron.purge-audit-log-days}") int purgeAuditLogDays, @Value("${cron.purge-expired-invitations-days}") int purgeExpiredInvitationDays) { - super(LOCK_NAME, dataSource); this.userRepository = userRepository; this.userRoleRepository = userRoleRepository; this.userRoleAuditRepository = userRoleAuditRepository; @@ -68,8 +65,10 @@ public ResourceCleaner(UserRepository userRepository, @Scheduled(cron = "${cron.user-cleaner-expression}") @Transactional + @SchedulerLock(name = LOCK_NAME, lockAtLeastFor = "${cron.user-cleaner-lock-at-least-for}", + lockAtMostFor = "${cron.user-cleaner-lock-at-most-for}") public void clean() { - super.perform("ResourceCleaner#clean", this::doClean); + this.doClean(); } public Map doClean() { diff --git a/server/src/main/java/invite/cron/RoleExpirationNotifier.java b/server/src/main/java/invite/cron/RoleExpirationNotifier.java index d9364710..a3821797 100644 --- a/server/src/main/java/invite/cron/RoleExpirationNotifier.java +++ b/server/src/main/java/invite/cron/RoleExpirationNotifier.java @@ -6,6 +6,7 @@ import invite.model.UserRole; import invite.repository.UserRoleRepository; import lombok.Getter; +import net.javacrumbs.shedlock.spring.annotation.SchedulerLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.Value; @@ -13,13 +14,12 @@ import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; -import javax.sql.DataSource; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.List; @Component -public class RoleExpirationNotifier extends AbstractNodeLeader { +public class RoleExpirationNotifier { public static final String LOCK_NAME = "role_expiration_notifier_user_level_lock"; private static final Log LOG = LogFactory.getLog(RoleExpirationNotifier.class); @@ -34,9 +34,7 @@ public class RoleExpirationNotifier extends AbstractNodeLeader { public RoleExpirationNotifier(UserRoleRepository userRoleRepository, Manage manage, MailBox mailBox, - DataSource dataSource, @Value("${cron.role-expiration-notifier-duration-days}") int roleExpirationNotificationDays) { - super(LOCK_NAME, dataSource); this.userRoleRepository = userRoleRepository; this.manage = manage; this.mailBox = mailBox; @@ -44,12 +42,14 @@ public RoleExpirationNotifier(UserRoleRepository userRoleRepository, } @Scheduled(cron = "${cron.role-expiration-notifier-expression}") + @SchedulerLock(name = LOCK_NAME, lockAtLeastFor = "${cron.role-expiration-notifier-lock-at-least-for}", + lockAtMostFor = "${cron.role-expiration-notifier-lock-at-most-for}") @Transactional public void sweep() { if (roleExpirationNotificationDays == -1) { return; } - super.perform("RoleExpirationNotifier#sweep", () -> this.doSweep()); + this.doSweep(); } public List doSweep() { diff --git a/server/src/main/java/invite/cron/ShedLockConfig.java b/server/src/main/java/invite/cron/ShedLockConfig.java new file mode 100644 index 00000000..168110ab --- /dev/null +++ b/server/src/main/java/invite/cron/ShedLockConfig.java @@ -0,0 +1,34 @@ +package invite.cron; + +import net.javacrumbs.shedlock.core.LockProvider; +import net.javacrumbs.shedlock.provider.jdbctemplate.JdbcTemplateLockProvider; +import net.javacrumbs.shedlock.spring.annotation.EnableSchedulerLock; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Profile; +import org.springframework.jdbc.core.JdbcTemplate; + +import javax.sql.DataSource; +import java.util.Optional; + +@Configuration +@EnableSchedulerLock(defaultLockAtMostFor = "PT10M") +public class ShedLockConfig { + + @Bean + @Profile("!test") + public LockProvider lockProvider(DataSource dataSource) { + return new JdbcTemplateLockProvider( + JdbcTemplateLockProvider.Configuration.builder() + .withJdbcTemplate(new JdbcTemplate(dataSource)) + .usingDbTime() // Use DB time, not app-node time — avoids clock skew + .build() + ); + } + + @Bean + @Profile("test") + public LockProvider noOpLockProvider() { + return lockConfiguration -> Optional.of(() -> {}); + } +} diff --git a/server/src/main/resources/application.yml b/server/src/main/resources/application.yml index d0819867..e6779d1b 100644 --- a/server/src/main/resources/application.yml +++ b/server/src/main/resources/application.yml @@ -79,10 +79,14 @@ crypto: cron: user-cleaner-expression: "0 0/30 * * * *" + user-cleaner-lock-at-least-for: "PT5M" + user-cleaner-lock-at-most-for: "PT28M" last-activity-duration-days: 1000 role-expiration-notifier-expression: "0 0/30 * * * *" # Set to -1 to suppress role expiry notifications role-expiration-notifier-duration-days: 5 + role-expiration-notifier-lock-at-least-for: "PT5M" + role-expiration-notifier-lock-at-most-for: "PT28M" metadata-resolver-initial-delay-milliseconds: 1 metadata-resolver-fixed-rate-milliseconds: 86_400_000 metadata-resolver-url: "classpath:/metadata/idps-metadata.xml" diff --git a/server/src/main/resources/db/mysql/migration/V58_0__shedlock_locking_table.sql b/server/src/main/resources/db/mysql/migration/V58_0__shedlock_locking_table.sql new file mode 100644 index 00000000..303c01f6 --- /dev/null +++ b/server/src/main/resources/db/mysql/migration/V58_0__shedlock_locking_table.sql @@ -0,0 +1,14 @@ +CREATE TABLE shedlock ( + name VARCHAR(64) NOT NULL, + lock_until TIMESTAMP(3) NOT NULL, + locked_at TIMESTAMP(3) NOT NULL, + locked_by VARCHAR(255) NOT NULL, + PRIMARY KEY (name) +) ENGINE=InnoDB + DEFAULT CHARSET = utf8mb4; + +-- Pre-insert your lock rows (critical for Galera safety) +INSERT INTO shedlock (name, lock_until, locked_at, locked_by) +VALUES ('resource_cleaner_user_level_lock', '2000-01-01 00:00:00.000', '2000-01-01 00:00:00.000', 'init'); +INSERT INTO shedlock (name, lock_until, locked_at, locked_by) +VALUES ('role_expiration_notifier_user_level_lock', '2000-01-01 00:00:00.000', '2000-01-01 00:00:00.000', 'init'); diff --git a/server/src/test/java/invite/AbstractTest.java b/server/src/test/java/invite/AbstractTest.java index cf5b5956..74c5bcbb 100644 --- a/server/src/test/java/invite/AbstractTest.java +++ b/server/src/test/java/invite/AbstractTest.java @@ -124,7 +124,8 @@ "manage.enabled: true", "spring.task.scheduling.enabled=false", "spring.jpa.properties.hibernate.format_sql=false", - "spring.jpa.show-sql=false" + "spring.jpa.show-sql=false", + "spring.main.allow-bean-definition-overriding=true" }) @SuppressWarnings("unchecked") public abstract class AbstractTest { diff --git a/server/src/test/java/invite/cron/ResourceCleanerTest.java b/server/src/test/java/invite/cron/ResourceCleanerTest.java index 6eb5ae3f..6a3b1ebb 100644 --- a/server/src/test/java/invite/cron/ResourceCleanerTest.java +++ b/server/src/test/java/invite/cron/ResourceCleanerTest.java @@ -5,18 +5,16 @@ import invite.model.Invitation; import invite.model.User; import invite.model.UserRoleAudit; -import lombok.SneakyThrows; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.context.annotation.Import; -import javax.sql.DataSource; -import java.sql.Connection; import java.time.Instant; import java.time.Period; import java.time.temporal.ChronoUnit; import java.util.List; -import static invite.cron.ResourceCleaner.LOCK_NAME; import static org.junit.jupiter.api.Assertions.assertEquals; class ResourceCleanerTest extends AbstractTest { @@ -24,9 +22,6 @@ class ResourceCleanerTest extends AbstractTest { @Autowired private ResourceCleaner resourceCleaner; - @Autowired - private DataSource dataSource; - @Test void cleanUsersWithoutActivity() throws JsonProcessingException { long beforeUsers = userRepository.count(); @@ -94,28 +89,14 @@ void cleanExpiredInvitations() { long count = invitationRepository.count(); Instant past = Instant.now().minus(400, ChronoUnit.DAYS); Invitation invitation = invitationRepository.findByHash(GRAPH_INVITATION_HASH).get(); - invitation.setExpiryDate(past); - invitationRepository.save(invitation); + invitation.setExpiryDate(past); + invitationRepository.save(invitation); resourceCleaner.clean(); assertEquals(count - 1, invitationRepository.count()); } - @SneakyThrows - @Test - void lockAlreadyAcquired() { - Connection conn = dataSource.getConnection(); - resourceCleaner.tryGetLock(conn, LOCK_NAME); - - long beforeUsers = userRepository.count(); - markUserAsVeryInactive(GUEST_SUB); - - resourceCleaner.clean(); - //Nothing happened - assertEquals(beforeUsers, userRepository.count()); - } - private void markUserAsVeryInactive(String sub) { User user = userRepository.findBySubIgnoreCase(sub).get(); Instant past = Instant.now().minus(Period.ofDays(1050)); diff --git a/server/src/test/java/invite/cron/ResourceCleanerUnitTest.java b/server/src/test/java/invite/cron/ResourceCleanerUnitTest.java index 3be86525..662c213d 100644 --- a/server/src/test/java/invite/cron/ResourceCleanerUnitTest.java +++ b/server/src/test/java/invite/cron/ResourceCleanerUnitTest.java @@ -40,7 +40,6 @@ class ResourceCleanerUnitTest { userRepository, userRoleRepository, provisioningService, - dataSource, userRoleAuditRepository, userRoleAuditService, invitationRepository, diff --git a/server/src/test/java/invite/cron/RoleExpirationNotifierTest.java b/server/src/test/java/invite/cron/RoleExpirationNotifierTest.java index 27fa95c2..a89bacd7 100644 --- a/server/src/test/java/invite/cron/RoleExpirationNotifierTest.java +++ b/server/src/test/java/invite/cron/RoleExpirationNotifierTest.java @@ -64,7 +64,7 @@ void sweepNonExistentGroupProvider() { @Test void noCronJobResponsible() { - RoleExpirationNotifier subject = new RoleExpirationNotifier(null, null, null, null, -1); + RoleExpirationNotifier subject = new RoleExpirationNotifier(null, null, null, -1); subject.sweep(); } } \ No newline at end of file diff --git a/welcome/pom.xml b/welcome/pom.xml index f97ac566..f399ce50 100644 --- a/welcome/pom.xml +++ b/welcome/pom.xml @@ -4,7 +4,7 @@ org.openconext invite - 1.1.1-SNAPSHOT + 1.1.2-SNAPSHOT ../pom.xml invite-welcome