Refactor SystemWideAlert to use ZonedDateTime to fix bugs discovered via SystemWideAlertRestRepositoryIT

This commit is contained in:
Tim Donohue
2025-02-25 11:27:56 -06:00
parent 03fd92a877
commit 710b0fa040
8 changed files with 45 additions and 42 deletions

View File

@@ -7,7 +7,7 @@
*/ */
package org.dspace.alerts; package org.dspace.alerts;
import java.time.LocalDateTime; import java.time.ZonedDateTime;
import jakarta.persistence.Cacheable; import jakarta.persistence.Cacheable;
import jakarta.persistence.Column; import jakarta.persistence.Column;
@@ -44,7 +44,7 @@ public class SystemWideAlert implements ReloadableEntity<Integer> {
private String allowSessions; private String allowSessions;
@Column(name = "countdown_to") @Column(name = "countdown_to")
private LocalDateTime countdownTo; private ZonedDateTime countdownTo;
@Column(name = "active") @Column(name = "active")
private boolean active; private boolean active;
@@ -112,7 +112,7 @@ public class SystemWideAlert implements ReloadableEntity<Integer> {
* *
* @return the date to which will be count down when the system-wide alert is active * @return the date to which will be count down when the system-wide alert is active
*/ */
public LocalDateTime getCountdownTo() { public ZonedDateTime getCountdownTo() {
return countdownTo; return countdownTo;
} }
@@ -121,7 +121,7 @@ public class SystemWideAlert implements ReloadableEntity<Integer> {
* *
* @param countdownTo The date to which will be count down * @param countdownTo The date to which will be count down
*/ */
public void setCountdownTo(final LocalDateTime countdownTo) { public void setCountdownTo(final ZonedDateTime countdownTo) {
this.countdownTo = countdownTo; this.countdownTo = countdownTo;
} }

View File

@@ -9,7 +9,7 @@ package org.dspace.alerts;
import java.io.IOException; import java.io.IOException;
import java.sql.SQLException; import java.sql.SQLException;
import java.time.LocalDateTime; import java.time.ZonedDateTime;
import java.util.List; import java.util.List;
import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Logger;
@@ -39,7 +39,7 @@ public class SystemWideAlertServiceImpl implements SystemWideAlertService {
@Override @Override
public SystemWideAlert create(final Context context, final String message, public SystemWideAlert create(final Context context, final String message,
final AllowSessionsEnum allowSessionsType, final AllowSessionsEnum allowSessionsType,
final LocalDateTime countdownTo, final boolean active) throws SQLException, final ZonedDateTime countdownTo, final boolean active) throws SQLException,
AuthorizeException { AuthorizeException {
if (!authorizeService.isAdmin(context)) { if (!authorizeService.isAdmin(context)) {
throw new AuthorizeException( throw new AuthorizeException(

View File

@@ -9,7 +9,7 @@ package org.dspace.alerts.service;
import java.io.IOException; import java.io.IOException;
import java.sql.SQLException; import java.sql.SQLException;
import java.time.LocalDateTime; import java.time.ZonedDateTime;
import java.util.List; import java.util.List;
import org.dspace.alerts.AllowSessionsEnum; import org.dspace.alerts.AllowSessionsEnum;
@@ -35,7 +35,7 @@ public interface SystemWideAlertService {
* @throws SQLException If something goes wrong * @throws SQLException If something goes wrong
*/ */
SystemWideAlert create(Context context, String message, AllowSessionsEnum allowSessionsType, SystemWideAlert create(Context context, String message, AllowSessionsEnum allowSessionsType,
LocalDateTime countdownTo, boolean active ZonedDateTime countdownTo, boolean active
) throws SQLException, AuthorizeException; ) throws SQLException, AuthorizeException;
/** /**

View File

@@ -8,7 +8,7 @@
package org.dspace.builder; package org.dspace.builder;
import java.sql.SQLException; import java.sql.SQLException;
import java.time.LocalDateTime; import java.time.ZonedDateTime;
import org.dspace.alerts.AllowSessionsEnum; import org.dspace.alerts.AllowSessionsEnum;
import org.dspace.alerts.SystemWideAlert; import org.dspace.alerts.SystemWideAlert;
@@ -31,7 +31,7 @@ public class SystemWideAlertBuilder extends AbstractBuilder<SystemWideAlert, Sys
} }
private SystemWideAlertBuilder create(Context context, String message, AllowSessionsEnum allowSessionsType, private SystemWideAlertBuilder create(Context context, String message, AllowSessionsEnum allowSessionsType,
LocalDateTime countdownTo, boolean active) ZonedDateTime countdownTo, boolean active)
throws SQLException, AuthorizeException { throws SQLException, AuthorizeException {
this.context = context; this.context = context;
this.systemWideAlert = systemWideAlertService.create(context, message, allowSessionsType, countdownTo, active); this.systemWideAlert = systemWideAlertService.create(context, message, allowSessionsType, countdownTo, active);
@@ -43,7 +43,7 @@ public class SystemWideAlertBuilder extends AbstractBuilder<SystemWideAlert, Sys
return this; return this;
} }
public SystemWideAlertBuilder withCountdownDate(LocalDateTime countdownTo) { public SystemWideAlertBuilder withCountdownDate(ZonedDateTime countdownTo) {
systemWideAlert.setCountdownTo(countdownTo); systemWideAlert.setCountdownTo(countdownTo);
return this; return this;
} }

View File

@@ -7,9 +7,10 @@
*/ */
package org.dspace.matcher; package org.dspace.matcher;
import java.time.LocalDateTime; import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter; import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException; import java.time.format.DateTimeParseException;
import java.time.temporal.ChronoUnit;
import org.hamcrest.BaseMatcher; import org.hamcrest.BaseMatcher;
import org.hamcrest.Description; import org.hamcrest.Description;
@@ -22,17 +23,18 @@ import org.hamcrest.Description;
*/ */
public class DateMatcher public class DateMatcher
extends BaseMatcher<String> { extends BaseMatcher<String> {
private static final DateTimeFormatter dateFormat private static final DateTimeFormatter dateFormat = DateTimeFormatter.ISO_ZONED_DATE_TIME;
= DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSX");
private final LocalDateTime matchDate; private final ZonedDateTime matchDate;
/** /**
* Create a matcher for a given Date. * Create a matcher for a given Date.
* @param matchDate The date that tested values should match. * @param matchDate The date that tested values should match.
*/ */
public DateMatcher(LocalDateTime matchDate) { public DateMatcher(ZonedDateTime matchDate) {
this.matchDate = matchDate; // Truncate to seconds. We aren't matching with millisecond precision because doing so may result
// in random failures if one of the two dates is rounded differently.
this.matchDate = (matchDate != null ? matchDate.truncatedTo(ChronoUnit.SECONDS) : null);
} }
@Override @Override
@@ -52,10 +54,10 @@ public class DateMatcher
throw new IllegalArgumentException("Argument not a String"); throw new IllegalArgumentException("Argument not a String");
} }
// Decode the string to a Date // Decode the string to a Date, truncated to seconds.
LocalDateTime testDateDecoded; ZonedDateTime testDateDecoded;
try { try {
testDateDecoded = LocalDateTime.parse((String) testDate, dateFormat); testDateDecoded = ZonedDateTime.parse((String) testDate, dateFormat).truncatedTo(ChronoUnit.SECONDS);
} catch (DateTimeParseException ex) { } catch (DateTimeParseException ex) {
throw new IllegalArgumentException("Argument '" + testDate throw new IllegalArgumentException("Argument '" + testDate
+ "' is not an ISO 8601 zoned date", ex); + "' is not an ISO 8601 zoned date", ex);
@@ -68,7 +70,7 @@ public class DateMatcher
@Override @Override
public void describeTo(Description description) { public void describeTo(Description description) {
description.appendText("is the same date as "); description.appendText("is the same date as ");
description.appendText(dateFormat.format(matchDate)); description.appendText(matchDate != null ? dateFormat.format(matchDate) : "null");
} }
/** /**
@@ -76,7 +78,7 @@ public class DateMatcher
* @param matchDate the date which tested values should match. * @param matchDate the date which tested values should match.
* @return a new Matcher for matchDate. * @return a new Matcher for matchDate.
*/ */
static public DateMatcher dateMatcher(LocalDateTime matchDate) { static public DateMatcher dateMatcher(ZonedDateTime matchDate) {
return new DateMatcher(matchDate); return new DateMatcher(matchDate);
} }
} }

View File

@@ -7,7 +7,7 @@
*/ */
package org.dspace.app.rest.model; package org.dspace.app.rest.model;
import java.time.LocalDateTime; import java.time.ZonedDateTime;
import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonProperty;
@@ -43,7 +43,7 @@ public class SystemWideAlertRest extends BaseObjectRest<Integer> {
private Integer alertId; private Integer alertId;
private String message; private String message;
private String allowSessions; private String allowSessions;
private LocalDateTime countdownTo; private ZonedDateTime countdownTo;
private boolean active; private boolean active;
public Integer getAlertId() { public Integer getAlertId() {
@@ -70,11 +70,11 @@ public class SystemWideAlertRest extends BaseObjectRest<Integer> {
this.allowSessions = allowSessions; this.allowSessions = allowSessions;
} }
public LocalDateTime getCountdownTo() { public ZonedDateTime getCountdownTo() {
return countdownTo; return countdownTo;
} }
public void setCountdownTo(final LocalDateTime countdownTo) { public void setCountdownTo(final ZonedDateTime countdownTo) {
this.countdownTo = countdownTo; this.countdownTo = countdownTo;
} }

View File

@@ -19,7 +19,8 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import java.time.LocalDateTime; import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReference;
import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper;
@@ -44,7 +45,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
// Create two alert entries in the db to fully test the findAll method // Create two alert entries in the db to fully test the findAll method
// Note: It is not possible to create two alerts through the REST API // Note: It is not possible to create two alerts through the REST API
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
LocalDateTime date = LocalDateTime.now(); ZonedDateTime date = ZonedDateTime.now(ZoneOffset.UTC);
SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1") SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1")
.withAllowSessions( .withAllowSessions(
AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY) AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY)
@@ -87,7 +88,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
// Create two alert entries in the db to fully test the findAll method // Create two alert entries in the db to fully test the findAll method
// Note: It is not possible to create two alerts through the REST API // Note: It is not possible to create two alerts through the REST API
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
LocalDateTime countdownDate = LocalDateTime.now(); ZonedDateTime countdownDate = ZonedDateTime.now(ZoneOffset.UTC);
SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1") SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1")
.withAllowSessions( .withAllowSessions(
AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY) AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY)
@@ -113,7 +114,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
// Create two alert entries in the db to fully test the findAll method // Create two alert entries in the db to fully test the findAll method
// Note: It is not possible to create two alerts through the REST API // Note: It is not possible to create two alerts through the REST API
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
LocalDateTime countdownDate = LocalDateTime.now(); ZonedDateTime countdownDate = ZonedDateTime.now(ZoneOffset.UTC);
SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1") SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1")
.withAllowSessions( .withAllowSessions(
AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY) AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY)
@@ -140,7 +141,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
// Create two alert entries in the db to fully test the findOne method // Create two alert entries in the db to fully test the findOne method
// Note: It is not possible to create two alerts through the REST API // Note: It is not possible to create two alerts through the REST API
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
LocalDateTime dateToNearestSecond = LocalDateTime.now(); ZonedDateTime dateToNearestSecond = ZonedDateTime.now(ZoneOffset.UTC);
SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1") SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1")
.withAllowSessions( .withAllowSessions(
AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY) AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY)
@@ -181,7 +182,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
// Create two alert entries in the db to fully test the findOne method // Create two alert entries in the db to fully test the findOne method
// Note: It is not possible to create two alerts through the REST API // Note: It is not possible to create two alerts through the REST API
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
LocalDateTime dateToNearestSecond = LocalDateTime.now(); ZonedDateTime dateToNearestSecond = ZonedDateTime.now(ZoneOffset.UTC);
SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1") SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1")
.withAllowSessions( .withAllowSessions(
AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY) AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY)
@@ -224,7 +225,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
// Create two alert entries in the db to fully test the findOne method // Create two alert entries in the db to fully test the findOne method
// Note: It is not possible to create two alerts through the REST API // Note: It is not possible to create two alerts through the REST API
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
LocalDateTime dateToNearestSecond = LocalDateTime.now(); ZonedDateTime dateToNearestSecond = ZonedDateTime.now(ZoneOffset.UTC);
SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1") SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1")
.withAllowSessions( .withAllowSessions(
AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY) AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY)
@@ -269,7 +270,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
// Create three alert entries in the db to fully test the findActive search method // Create three alert entries in the db to fully test the findActive search method
// Note: It is not possible to create two alerts through the REST API // Note: It is not possible to create two alerts through the REST API
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
LocalDateTime dateToNearestSecond = LocalDateTime.now(); ZonedDateTime dateToNearestSecond = ZonedDateTime.now(ZoneOffset.UTC);
SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1") SystemWideAlert systemWideAlert1 = SystemWideAlertBuilder.createSystemWideAlert(context, "Test alert 1")
.withAllowSessions( .withAllowSessions(
AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY) AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY)
@@ -316,7 +317,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
@Test @Test
public void createTest() throws Exception { public void createTest() throws Exception {
LocalDateTime dateToNearestSecond = LocalDateTime.now(); ZonedDateTime dateToNearestSecond = ZonedDateTime.now(ZoneOffset.UTC);
SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest(); SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest();
systemWideAlertRest.setMessage("Alert test message"); systemWideAlertRest.setMessage("Alert test message");
@@ -367,7 +368,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest(); SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest();
systemWideAlertRest.setMessage("Alert test message"); systemWideAlertRest.setMessage("Alert test message");
systemWideAlertRest.setCountdownTo(LocalDateTime.now()); systemWideAlertRest.setCountdownTo(ZonedDateTime.now(ZoneOffset.UTC));
systemWideAlertRest.setAllowSessions(AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY.getValue()); systemWideAlertRest.setAllowSessions(AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY.getValue());
systemWideAlertRest.setActive(true); systemWideAlertRest.setActive(true);
@@ -385,7 +386,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest(); SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest();
systemWideAlertRest.setMessage("Alert test message"); systemWideAlertRest.setMessage("Alert test message");
systemWideAlertRest.setCountdownTo(LocalDateTime.now()); systemWideAlertRest.setCountdownTo(ZonedDateTime.now(ZoneOffset.UTC));
systemWideAlertRest.setAllowSessions(AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY.getValue()); systemWideAlertRest.setAllowSessions(AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY.getValue());
systemWideAlertRest.setActive(true); systemWideAlertRest.setActive(true);
@@ -412,7 +413,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest(); SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest();
systemWideAlertRest.setMessage("Alert test message"); systemWideAlertRest.setMessage("Alert test message");
systemWideAlertRest.setCountdownTo(LocalDateTime.now()); systemWideAlertRest.setCountdownTo(ZonedDateTime.now(ZoneOffset.UTC));
systemWideAlertRest.setAllowSessions(AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY.getValue()); systemWideAlertRest.setAllowSessions(AllowSessionsEnum.ALLOW_CURRENT_SESSIONS_ONLY.getValue());
systemWideAlertRest.setActive(true); systemWideAlertRest.setActive(true);
@@ -436,7 +437,7 @@ public class SystemWideAlertRestRepositoryIT extends AbstractControllerIntegrati
.build(); .build();
context.restoreAuthSystemState(); context.restoreAuthSystemState();
LocalDateTime dateToNearestSecond = LocalDateTime.now(); ZonedDateTime dateToNearestSecond = ZonedDateTime.now(ZoneOffset.UTC);
SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest(); SystemWideAlertRest systemWideAlertRest = new SystemWideAlertRest();
systemWideAlertRest.setAlertId(systemWideAlert.getID()); systemWideAlertRest.setAlertId(systemWideAlert.getID());

View File

@@ -36,12 +36,12 @@ public class RequestCopyMatcher {
hasJsonPath("$.requestName", is(request.getReqName())), hasJsonPath("$.requestName", is(request.getReqName())),
hasJsonPath("$.requestMessage", is(request.getReqMessage())), hasJsonPath("$.requestMessage", is(request.getReqMessage())),
hasJsonPath("$.requestDate", dateMatcher(request.getRequest_date() hasJsonPath("$.requestDate", dateMatcher(request.getRequest_date()
.atZone(ZoneOffset.UTC).toLocalDateTime())), .atZone(ZoneOffset.UTC))),
hasJsonPath("$.acceptRequest", is(request.isAccept_request())), hasJsonPath("$.acceptRequest", is(request.isAccept_request())),
hasJsonPath("$.decisionDate", dateMatcher(request.getDecision_date() hasJsonPath("$.decisionDate", dateMatcher(request.getDecision_date()
.atZone(ZoneOffset.UTC).toLocalDateTime())), .atZone(ZoneOffset.UTC))),
hasJsonPath("$.expires", dateMatcher(request.getExpires() hasJsonPath("$.expires", dateMatcher(request.getExpires()
.atZone(ZoneOffset.UTC).toLocalDateTime())), .atZone(ZoneOffset.UTC))),
hasJsonPath("$.type", is(RequestItemRest.NAME)), hasJsonPath("$.type", is(RequestItemRest.NAME)),
hasJsonPath("$._links.self.href", hasJsonPath("$._links.self.href",
Matchers.containsString(RequestItemRepositoryIT.URI_ROOT)) Matchers.containsString(RequestItemRepositoryIT.URI_ROOT))