This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new 5deb6a958df Improve date/timestamp handling performance in
GenericDaoBase and DateUtil (#13809)
5deb6a958df is described below
commit 5deb6a958df549b68232f9e7dedff8de4422e603
Author: Manoj Kumar <[email protected]>
AuthorDate: Thu Oct 1 03:30:06 2026 +0530
Improve date/timestamp handling performance in GenericDaoBase and DateUtil
(#13809)
* Improve date/timestamp handling performance in GenericDaoBase and DateUtil
* address review comment
---
.../org/apache/cloudstack/backup/BackupVO.java | 2 +-
.../java/com/cloud/utils/db/GenericDaoBase.java | 78 +++++++++-----
.../com/cloud/utils/db/GenericDaoBaseTest.java | 75 ++++++++++++++
server/src/main/java/com/cloud/api/ApiServer.java | 7 +-
.../src/test/java/com/cloud/api/ApiServerTest.java | 69 +++++++++++++
utils/src/main/java/com/cloud/utils/DateUtil.java | 63 +++++-------
.../test/java/com/cloud/utils/DateUtilTest.java | 113 +++++++++++++++++++--
7 files changed, 330 insertions(+), 77 deletions(-)
diff --git
a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java
b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java
index b4cd2f7bada..a11de51ed63 100644
--- a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java
+++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java
@@ -60,7 +60,7 @@ public class BackupVO implements Backup {
private String backupType;
@Column(name = "date")
- @Temporal(value = TemporalType.DATE)
+ @Temporal(value = TemporalType.TIMESTAMP)
private Date date;
@Column(name = GenericDao.REMOVED_COLUMN)
diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java
b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java
index e6a4bb8f9b9..3290d7125f2 100644
--- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java
+++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java
@@ -34,6 +34,9 @@ import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.sql.Statement;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.sql.Types;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
@@ -67,7 +70,6 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import com.amazonaws.util.CollectionUtils;
-import com.cloud.utils.DateUtil;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.Ternary;
@@ -129,6 +131,28 @@ public abstract class GenericDaoBase<T, ID extends
Serializable> extends Compone
protected final static TimeZone s_gmtTimeZone =
TimeZone.getTimeZone("GMT");
+ /**
+ * Returns a fresh GMT {@link Calendar} for a single JDBC get/set
timestamp call. Calendar is
+ * mutable and JDBC drivers may mutate the instance passed to them, so a
new one is used per call
+ * rather than sharing a single instance across concurrent DAO operations.
+ */
+ protected static Calendar gmtCalendar() {
+ return Calendar.getInstance(s_gmtTimeZone);
+ }
+
+ /**
+ * Returns the SQL type ({@link Types}) matching the temporal flag of the
given attribute, so a
+ * null date/time/timestamp column is bound with the correct type instead
of always TIMESTAMP.
+ */
+ protected static int temporalSqlType(Attribute attr) {
+ if (attr.is(Attribute.Flag.Date)) {
+ return Types.DATE;
+ } else if (attr.is(Attribute.Flag.Time)) {
+ return Types.TIME;
+ }
+ return Types.TIMESTAMP;
+ }
+
protected final static Map<Class<?>, GenericDao<?, ? extends
Serializable>> s_daoMaps = new ConcurrentHashMap<Class<?>, GenericDao<?, ?
extends Serializable>>(71);
private final ConversionSupport _conversionSupport;
@@ -598,20 +622,16 @@ public abstract class GenericDaoBase<T, ID extends
Serializable> extends Compone
field.set(entity, rs.getInt(index));
}
} else if (type == Date.class) {
- final Object data = rs.getDate(index);
- if (data == null) {
- field.set(entity, null);
- return;
- }
- field.set(entity, DateUtil.parseDateString(s_gmtTimeZone,
rs.getString(index)));
+ final Timestamp ts = rs.getTimestamp(index, gmtCalendar());
+ field.set(entity, ts == null ? null : new Date(ts.getTime()));
} else if (type == Calendar.class) {
- final Object data = rs.getDate(index);
+ final Timestamp data = rs.getTimestamp(index, gmtCalendar());
if (data == null) {
field.set(entity, null);
return;
}
- final Calendar cal = Calendar.getInstance();
- cal.setTime(DateUtil.parseDateString(s_gmtTimeZone,
rs.getString(index)));
+ final Calendar cal = Calendar.getInstance(s_gmtTimeZone);
+ cal.setTime(data);
field.set(entity, cal);
} else if (type == boolean.class) {
field.setBoolean(entity, rs.getBoolean(index));
@@ -732,11 +752,11 @@ public abstract class GenericDaoBase<T, ID extends
Serializable> extends Compone
return (M) (Long) rs.getLong(index);
}
} else if (type == Date.class) {
- final Object data = rs.getDate(index);
- if (data == null) {
+ final Timestamp ts = rs.getTimestamp(index, gmtCalendar());
+ if (ts == null) {
return null;
} else {
- return (M)DateUtil.parseDateString(s_gmtTimeZone,
rs.getString(index));
+ return (M) new Date(ts.getTime());
}
} else if (type == short.class) {
return (M) (Short) rs.getShort(index);
@@ -779,12 +799,12 @@ public abstract class GenericDaoBase<T, ID extends
Serializable> extends Compone
return (M) (Byte) rs.getByte(index);
}
} else if (type == Calendar.class) {
- final Object data = rs.getDate(index);
+ final Timestamp data = rs.getTimestamp(index, gmtCalendar());
if (data == null) {
return null;
} else {
- final Calendar cal = Calendar.getInstance();
- cal.setTime(DateUtil.parseDateString(s_gmtTimeZone,
rs.getString(index)));
+ final Calendar cal = Calendar.getInstance(s_gmtTimeZone);
+ cal.setTime(data);
return (M)cal;
}
} else if (type == byte[].class) {
@@ -1696,7 +1716,12 @@ public abstract class GenericDaoBase<T, ID extends
Serializable> extends Compone
while (en.hasMoreElements()) {
pstmt = txn.prepareAutoCloseStatement(ec.insertSql);
if (ec.targetClass == Date.class) {
- pstmt.setString(1,
DateUtil.getDateDisplayString(s_gmtTimeZone, (Date)en.nextElement()));
+ Date d = (Date) en.nextElement();
+ if (d == null) {
+ pstmt.setNull(1, Types.TIMESTAMP);
+ } else {
+ pstmt.setTimestamp(1, new Timestamp(d.getTime()),
gmtCalendar());
+ }
} else {
pstmt.setObject(1, en.nextElement());
}
@@ -1800,28 +1825,28 @@ public abstract class GenericDaoBase<T, ID extends
Serializable> extends Compone
} else if (attr.field.getType() == Date.class) {
final Date date = (Date)value;
if (date == null || date.equals(DATE_TO_NULL)) {
- pstmt.setObject(j, null);
+ pstmt.setNull(j, temporalSqlType(attr));
return;
}
if (attr.is(Attribute.Flag.Date)) {
- pstmt.setString(j,
DateUtil.getDateDisplayString(s_gmtTimeZone, date));
+ pstmt.setDate(j, new java.sql.Date(date.getTime()),
gmtCalendar());
} else if (attr.is(Attribute.Flag.TimeStamp)) {
- pstmt.setString(j,
DateUtil.getDateDisplayString(s_gmtTimeZone, date));
+ pstmt.setTimestamp(j, new Timestamp(date.getTime()),
gmtCalendar());
} else if (attr.is(Attribute.Flag.Time)) {
- pstmt.setString(j,
DateUtil.getDateDisplayString(s_gmtTimeZone, date));
+ pstmt.setTime(j, new java.sql.Time(date.getTime()),
gmtCalendar());
}
} else if (attr.field.getType() == Calendar.class) {
final Calendar cal = (Calendar)value;
if (cal == null) {
- pstmt.setObject(j, null);
+ pstmt.setNull(j, temporalSqlType(attr));
return;
}
if (attr.is(Attribute.Flag.Date)) {
- pstmt.setString(j,
DateUtil.getDateDisplayString(s_gmtTimeZone, cal.getTime()));
+ pstmt.setDate(j, new java.sql.Date(cal.getTimeInMillis()),
gmtCalendar());
} else if (attr.is(Attribute.Flag.TimeStamp)) {
- pstmt.setString(j,
DateUtil.getDateDisplayString(s_gmtTimeZone, cal.getTime()));
+ pstmt.setTimestamp(j, new Timestamp(cal.getTimeInMillis()),
gmtCalendar());
} else if (attr.is(Attribute.Flag.Time)) {
- pstmt.setString(j,
DateUtil.getDateDisplayString(s_gmtTimeZone, cal.getTime()));
+ pstmt.setTime(j, new Time(cal.getTimeInMillis()),
gmtCalendar());
}
} else if (attr.field.getType().isEnum()) {
final Enumerated enumerated =
attr.field.getAnnotation(Enumerated.class);
@@ -1955,7 +1980,8 @@ public abstract class GenericDaoBase<T, ID extends
Serializable> extends Compone
}
} else if (ec.targetClass == Date.class) {
while (rs.next()) {
- lst.add(DateUtil.parseDateString(s_gmtTimeZone,
rs.getString(1)));
+ final Timestamp ts = rs.getTimestamp(1, gmtCalendar());
+ lst.add(ts == null ? null : new Date(ts.getTime()));
}
} else if (ec.targetClass == Boolean.class) {
while (rs.next()) {
diff --git
a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java
b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java
index ebf514f532f..41a7be4ab86 100644
--- a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java
+++ b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java
@@ -18,9 +18,14 @@ package com.cloud.utils.db;
import java.sql.ResultSet;
import java.sql.SQLException;
+import java.sql.Timestamp;
+import java.sql.Types;
import java.util.ArrayList;
+import java.util.Calendar;
import java.util.Collection;
+import java.util.Date;
import java.util.List;
+import java.util.TimeZone;
import org.junit.Assert;
import org.junit.Before;
@@ -331,4 +336,74 @@ public class GenericDaoBaseTest {
Assert.assertNotNull(result);
Assert.assertEquals(expectedResult, result);
}
+
+ @Test
+ public void gmtCalendarUsesGmtTimeZone() {
+ Calendar calendar = GenericDaoBase.gmtCalendar();
+
+ Assert.assertEquals(TimeZone.getTimeZone("GMT"),
calendar.getTimeZone());
+ }
+
+ @Test
+ public void gmtCalendarReturnsFreshInstancePerCall() {
+ Assert.assertNotSame(GenericDaoBase.gmtCalendar(),
GenericDaoBase.gmtCalendar());
+ }
+
+ @Test
+ public void temporalSqlTypeDate() {
+ Attribute attr = new Attribute("table", "column");
+ attr.flags = Attribute.Flag.Date.setTrue(attr.flags);
+
+ Assert.assertEquals(Types.DATE, GenericDaoBase.temporalSqlType(attr));
+ }
+
+ @Test
+ public void temporalSqlTypeTime() {
+ Attribute attr = new Attribute("table", "column");
+ attr.flags = Attribute.Flag.Time.setTrue(attr.flags);
+
+ Assert.assertEquals(Types.TIME, GenericDaoBase.temporalSqlType(attr));
+ }
+
+ @Test
+ public void temporalSqlTypeDefaultsToTimestamp() {
+ Attribute attr = new Attribute("table", "column");
+ attr.flags = Attribute.Flag.TimeStamp.setTrue(attr.flags);
+
+ Assert.assertEquals(Types.TIMESTAMP,
GenericDaoBase.temporalSqlType(attr));
+ }
+
+ @Test
+ public void getObjectDateReadsViaGmtTimestamp() throws SQLException {
+ Timestamp ts = new Timestamp(1_700_000_000_000L);
+ Mockito.when(resultSet.getTimestamp(Mockito.eq(2),
Mockito.any(Calendar.class))).thenReturn(ts);
+
+ Date result = GenericDaoBase.getObject(Date.class, resultSet, 2);
+
+ Assert.assertEquals(ts.getTime(), result.getTime());
+ }
+
+ @Test
+ public void getObjectDateNullTimestampReturnsNull() throws SQLException {
+ Mockito.when(resultSet.getTimestamp(Mockito.eq(3),
Mockito.any(Calendar.class))).thenReturn(null);
+
+ Assert.assertNull(GenericDaoBase.getObject(Date.class, resultSet, 3));
+ }
+
+ @Test
+ public void getObjectCalendarReadsViaGmtTimestamp() throws SQLException {
+ Timestamp ts = new Timestamp(1_700_000_000_000L);
+ Mockito.when(resultSet.getTimestamp(Mockito.eq(4),
Mockito.any(Calendar.class))).thenReturn(ts);
+
+ Calendar result = GenericDaoBase.getObject(Calendar.class, resultSet,
4);
+
+ Assert.assertEquals(ts.getTime(), result.getTimeInMillis());
+ }
+
+ @Test
+ public void getObjectCalendarNullTimestampReturnsNull() throws
SQLException {
+ Mockito.when(resultSet.getTimestamp(Mockito.eq(5),
Mockito.any(Calendar.class))).thenReturn(null);
+
+ Assert.assertNull(GenericDaoBase.getObject(Calendar.class, resultSet,
5));
+ }
}
diff --git a/server/src/main/java/com/cloud/api/ApiServer.java
b/server/src/main/java/com/cloud/api/ApiServer.java
index 6e058746ccb..396ec505948 100644
--- a/server/src/main/java/com/cloud/api/ApiServer.java
+++ b/server/src/main/java/com/cloud/api/ApiServer.java
@@ -192,6 +192,7 @@ import com.google.gson.reflect.TypeToken;
import static com.cloud.user.AccountManagerImpl.apiKeyAccess;
import static
org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled;
+import static org.apache.commons.lang3.StringUtils.deleteWhitespace;
@Component
public class ApiServer extends ManagerBase implements HttpRequestHandler,
ApiServerService, Configurable {
@@ -1372,11 +1373,10 @@ public class ApiServer extends ManagerBase implements
HttpRequestHandler, ApiSer
throw new PermissionDeniedException("User is null for role based
API access check for command" + commandName);
}
- final Account account = accountMgr.getAccount(user.getAccountId());
- final String accessAllowedCidrs =
ApiServiceConfiguration.ApiAllowedSourceCidrList.valueIn(account.getId()).replaceAll("\\s","");
final Boolean apiSourceCidrChecksEnabled =
ApiServiceConfiguration.ApiSourceCidrChecksEnabled.value();
-
if (apiSourceCidrChecksEnabled) {
+ final Account account = accountMgr.getAccount(user.getAccountId());
+ final String accessAllowedCidrs =
deleteWhitespace(ApiServiceConfiguration.ApiAllowedSourceCidrList.valueIn(account.getId()));
logger.debug("CIDRs from which account '" + account.toString() +
"' is allowed to perform API calls: " + accessAllowedCidrs);
if (!NetUtils.isIpInCidrList(remoteAddress,
accessAllowedCidrs.split(","))) {
logger.warn("Request by account '" + account.toString() + "'
was denied since " + remoteAddress + " does not match " + accessAllowedCidrs);
@@ -1384,7 +1384,6 @@ public class ApiServer extends ManagerBase implements
HttpRequestHandler, ApiSer
}
}
-
for (final APIChecker apiChecker : apiAccessCheckers) {
apiChecker.checkAccess(user, commandName);
}
diff --git a/server/src/test/java/com/cloud/api/ApiServerTest.java
b/server/src/test/java/com/cloud/api/ApiServerTest.java
index dedd6e02ec5..8ecdc458018 100644
--- a/server/src/test/java/com/cloud/api/ApiServerTest.java
+++ b/server/src/test/java/com/cloud/api/ApiServerTest.java
@@ -17,14 +17,20 @@
package com.cloud.api;
import com.cloud.domain.Domain;
+import com.cloud.exception.OriginDeniedException;
+import com.cloud.exception.PermissionDeniedException;
import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
import com.cloud.user.User;
import com.cloud.user.UserAccount;
import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.cloudstack.config.ApiServiceConfiguration;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.user.UserPasswordResetManager;
+import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
+import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -33,9 +39,12 @@ import org.mockito.Mock;
import org.mockito.MockedConstruction;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.test.util.ReflectionTestUtils;
import java.lang.reflect.Field;
+import java.net.InetAddress;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import static
org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled;
@@ -49,6 +58,12 @@ public class ApiServerTest {
@Mock
UserPasswordResetManager userPasswordResetManager;
+ @Mock
+ AccountManager accountMgr;
+
+ private static final String DEFAULT_CIDR_CHECKS_ENABLED =
ApiServiceConfiguration.ApiSourceCidrChecksEnabled.defaultValue();
+ private static final String DEFAULT_ALLOWED_CIDRS =
ApiServiceConfiguration.ApiAllowedSourceCidrList.defaultValue();
+
@BeforeClass
public static void beforeClass() throws Exception {
overrideDefaultConfigValue(UserPasswordResetEnabled, "_value", true);
@@ -59,6 +74,17 @@ public class ApiServerTest {
overrideDefaultConfigValue(UserPasswordResetEnabled, "_value", false);
}
+ @Before
+ public void setupCommandAvailableChecks() {
+ apiServer.setApiAccessCheckers(Collections.emptyList());
+ }
+
+ @After
+ public void resetCidrConfig() throws Exception {
+
overrideDefaultConfigValue(ApiServiceConfiguration.ApiSourceCidrChecksEnabled,
"_defaultValue", DEFAULT_CIDR_CHECKS_ENABLED);
+
overrideDefaultConfigValue(ApiServiceConfiguration.ApiAllowedSourceCidrList,
"_defaultValue", DEFAULT_ALLOWED_CIDRS);
+ }
+
private static void overrideDefaultConfigValue(final ConfigKey configKey,
final String name, final Object o) throws IllegalAccessException,
NoSuchFieldException {
Field f = ConfigKey.class.getDeclaredField(name);
f.setAccessible(true);
@@ -176,4 +202,47 @@ public class ApiServerTest {
Mockito.when(account.getApiKeyAccess()).thenReturn(null);
Assert.assertEquals(true, apiServer.verifyApiKeyAccessAllowed(user,
account));
}
+
+ @Test
+ public void testCheckCommandAvailableSkipsCidrLookupWhenDisabled() throws
Exception {
+
overrideDefaultConfigValue(ApiServiceConfiguration.ApiSourceCidrChecksEnabled,
"_defaultValue", "false");
+ User user = Mockito.mock(User.class);
+
+ ReflectionTestUtils.invokeMethod(apiServer, "checkCommandAvailable",
user, "listVirtualMachines", InetAddress.getByName("127.0.0.1"));
+
+ Mockito.verify(accountMgr,
Mockito.never()).getAccount(Mockito.anyLong());
+ }
+
+ @Test
+ public void testCheckCommandAvailableAllowsMatchingCidr() throws Exception
{
+
overrideDefaultConfigValue(ApiServiceConfiguration.ApiSourceCidrChecksEnabled,
"_defaultValue", "true");
+
overrideDefaultConfigValue(ApiServiceConfiguration.ApiAllowedSourceCidrList,
"_defaultValue", "127.0.0.1/32");
+ User user = Mockito.mock(User.class);
+ Mockito.when(user.getAccountId()).thenReturn(1L);
+ Account account = Mockito.mock(Account.class);
+ Mockito.when(account.getId()).thenReturn(1L);
+ Mockito.when(accountMgr.getAccount(1L)).thenReturn(account);
+
+ ReflectionTestUtils.invokeMethod(apiServer, "checkCommandAvailable",
user, "listVirtualMachines", InetAddress.getByName("127.0.0.1"));
+
+ Mockito.verify(accountMgr).getAccount(1L);
+ }
+
+ @Test(expected = OriginDeniedException.class)
+ public void testCheckCommandAvailableDeniesNonMatchingCidr() throws
Exception {
+
overrideDefaultConfigValue(ApiServiceConfiguration.ApiSourceCidrChecksEnabled,
"_defaultValue", "true");
+
overrideDefaultConfigValue(ApiServiceConfiguration.ApiAllowedSourceCidrList,
"_defaultValue", "10.0.0.0/8");
+ User user = Mockito.mock(User.class);
+ Mockito.when(user.getAccountId()).thenReturn(1L);
+ Account account = Mockito.mock(Account.class);
+ Mockito.when(account.getId()).thenReturn(1L);
+ Mockito.when(accountMgr.getAccount(1L)).thenReturn(account);
+
+ ReflectionTestUtils.invokeMethod(apiServer, "checkCommandAvailable",
user, "listVirtualMachines", InetAddress.getByName("127.0.0.1"));
+ }
+
+ @Test(expected = PermissionDeniedException.class)
+ public void testCheckCommandAvailableThrowsWhenUserNull() throws Exception
{
+ ReflectionTestUtils.invokeMethod(apiServer, "checkCommandAvailable",
null, "listVirtualMachines", InetAddress.getByName("127.0.0.1"));
+ }
}
diff --git a/utils/src/main/java/com/cloud/utils/DateUtil.java
b/utils/src/main/java/com/cloud/utils/DateUtil.java
index 00ae5565dad..73036285cdc 100644
--- a/utils/src/main/java/com/cloud/utils/DateUtil.java
+++ b/utils/src/main/java/com/cloud/utils/DateUtil.java
@@ -19,17 +19,16 @@
package com.cloud.utils;
-import java.text.DateFormat;
import java.text.ParseException;
-import java.text.SimpleDateFormat;
-import java.time.LocalDateTime;
-import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.YearMonth;
import java.util.Calendar;
import java.util.Date;
import java.util.TimeZone;
+import java.util.concurrent.ConcurrentHashMap;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;
import java.time.OffsetDateTime;
@@ -49,7 +48,11 @@ public class DateUtil {
public static final TimeZone GMT_TIMEZONE = TimeZone.getTimeZone("GMT");
public static final String YYYYMMDD_FORMAT = "yyyyMMddHHmmss";
public static final String ZONED_DATETIME_FORMAT =
"yyyy-MM-dd'T'HH:mm:ssZ";
- private static final DateFormat ZONED_DATETIME_SIMPLE_FORMATTER = new
SimpleDateFormat(ZONED_DATETIME_FORMAT);
+
+ private static final DateTimeFormatter OUTPUT_FORMATTER =
+
DateTimeFormatter.ofPattern(ZONED_DATETIME_FORMAT).withZone(ZoneId.systemDefault());
+
+ private static final ConcurrentHashMap<String, DateTimeFormatter>
s_formatterCache = new ConcurrentHashMap<>();
private static final DateTimeFormatter[] parseFormats = new
DateTimeFormatter[]{
DateTimeFormatter.ISO_OFFSET_DATE_TIME,
@@ -68,6 +71,11 @@ public class DateUtil {
return new Date();
}
+ private static DateTimeFormatter getFormatter(String pattern, ZoneId zone)
{
+ String key = pattern + "|" + zone.getId();
+ return s_formatterCache.computeIfAbsent(key, k ->
DateTimeFormatter.ofPattern(pattern).withZone(zone));
+ }
+
public static Date parseTZDateString(String str) throws ParseException {
for (DateTimeFormatter formatter : parseFormats) {
try {
@@ -85,13 +93,12 @@ public class DateUtil {
}
public static Date parseDateString(TimeZone tz, String dateString, String
formatString) {
- DateFormat df = new SimpleDateFormat(formatString);
- df.setTimeZone(tz);
-
+ ZoneId zoneId = tz.toZoneId();
+ DateTimeFormatter formatter = getFormatter(formatString, zoneId);
try {
- return df.parse(dateString);
- } catch (ParseException e) {
- throw new CloudRuntimeException("why why ", e);
+ return Date.from(LocalDateTime.parse(dateString,
formatter).atZone(zoneId).toInstant());
+ } catch (DateTimeParseException e) {
+ throw new CloudRuntimeException("Failed to parse date string: " +
dateString, e);
}
}
@@ -108,21 +115,14 @@ public class DateUtil {
return null;
}
- DateFormat df = new SimpleDateFormat(formatString);
- df.setTimeZone(tz);
-
- return df.format(time);
+ return getFormatter(formatString,
tz.toZoneId()).format(time.toInstant());
}
public static String getOutputString(Date date) {
if (date == null) {
return "";
}
- String formattedString;
- synchronized (ZONED_DATETIME_SIMPLE_FORMATTER) {
- formattedString = ZONED_DATETIME_SIMPLE_FORMATTER.format(date);
- }
- return formattedString;
+ return OUTPUT_FORMATTER.format(date.toInstant());
}
public static Date now() {
@@ -155,7 +155,7 @@ public class DateUtil {
/**
* Return next run time
- * @param intervalType hourly/daily/weekly/monthly
+ * @param type hourly/daily/weekly/monthly
* @param schedule MM[:HH][:DD] format. DD is day of week for weekly and
day of month for monthly
* @param timezone The timezone in which the schedule string is specified
* @param startDate if specified, returns next run time after the
specified startDate
@@ -177,7 +177,8 @@ public class DateUtil {
int minutes = 0;
int hour = 0;
int day = 0;
- Date execDate = null;
+ Date execDate;
+ Date now = new Date();
switch (type) {
case HOURLY:
@@ -199,7 +200,7 @@ public class DateUtil {
// During testing we use a test clock which runs much faster
than the real clock
// So startDate and execDate will always be ahead in the future
// and we will never increase the time here
- if (execDate.before(new Date()) || !execDate.after(startDate))
{
+ if (execDate.before(now) || !execDate.after(startDate)) {
scheduleTime.add(Calendar.HOUR_OF_DAY, 1);
}
break;
@@ -225,7 +226,7 @@ public class DateUtil {
// During testing we use a test clock which runs much faster
than the real clock
// So startDate and execDate will always be ahead in the future
// and we will never increase the time here
- if (execDate.before(new Date()) || !execDate.after(startDate))
{
+ if (execDate.before(now) || !execDate.after(startDate)) {
scheduleTime.add(Calendar.DAY_OF_YEAR, 1);
}
break;
@@ -252,10 +253,9 @@ public class DateUtil {
// During testing we use a test clock which runs much faster
than the real clock
// So startDate and execDate will always be ahead in the future
// and we will never increase the time here
- if (execDate.before(new Date()) || !execDate.after(startDate))
{
+ if (execDate.before(now) || !execDate.after(startDate)) {
scheduleTime.add(Calendar.DAY_OF_WEEK, 7);
}
- ;
break;
case MONTHLY:
if (scheduleParts.length < 3) {
@@ -283,7 +283,7 @@ public class DateUtil {
// During testing we use a test clock which runs much faster
than the real clock
// So startDate and execDate will always be ahead in the future
// and we will never increase the time here
- if (execDate.before(new Date()) || !execDate.after(startDate))
{
+ if (execDate.before(now) || !execDate.after(startDate)) {
scheduleTime.add(Calendar.MONTH, 1);
}
break;
@@ -302,14 +302,7 @@ public class DateUtil {
}
public static long getTimeDifference(Date date1, Date date2){
-
- Calendar dateCalendar1 = Calendar.getInstance();
- dateCalendar1.setTime(date1);
- Calendar dateCalendar2 = Calendar.getInstance();
- dateCalendar2.setTime(date2);
-
- return (dateCalendar1.getTimeInMillis() -
dateCalendar2.getTimeInMillis() )/1000;
-
+ return (date1.getTime() - date2.getTime()) / 1000;
}
public static CronExpression parseSchedule(String schedule) {
diff --git a/utils/src/test/java/com/cloud/utils/DateUtilTest.java
b/utils/src/test/java/com/cloud/utils/DateUtilTest.java
index 98b4d11c9d7..77079986cbe 100644
--- a/utils/src/test/java/com/cloud/utils/DateUtilTest.java
+++ b/utils/src/test/java/com/cloud/utils/DateUtilTest.java
@@ -18,30 +18,36 @@
//
package com.cloud.utils;
+import com.cloud.utils.DateUtil.IntervalType;
+import com.cloud.utils.exception.CloudRuntimeException;
+import org.junit.Test;
+
import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.time.Instant;
+import java.time.OffsetDateTime;
import java.time.ZoneId;
+import java.time.format.DateTimeFormatter;
import java.util.Calendar;
import java.util.Date;
import java.util.TimeZone;
-
-import java.time.format.DateTimeFormatter;
-import java.time.OffsetDateTime;
-
-import com.cloud.utils.DateUtil.IntervalType;
-
-import org.junit.Test;
-
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
public class DateUtilTest {
+ private static final String TEST_DATE_GMT = "2023-06-15 10:30:00";
+ private static final String TEST_DATE_EST = "2023-06-15 05:30:00";
+ private static final String TEST_DATE_ISO = "2023-06-15T10:30:00Z";
+ private static final String TEST_DATE_YYMMDD = "20230615103000";
+ private static final TimeZone GMT = DateUtil.GMT_TIMEZONE;
+ private static final TimeZone EST = TimeZone.getTimeZone("EST");
+
// command line test tool
public static void main(String[] args) {
TimeZone localTimezone = Calendar.getInstance().getTimeZone();
- TimeZone gmtTimezone = TimeZone.getTimeZone("GMT");
- TimeZone estTimezone = TimeZone.getTimeZone("EST");
+ TimeZone gmtTimezone = GMT;
+ TimeZone estTimezone = EST;
Date time = new Date();
System.out.println("local time :" +
DateUtil.getDateDisplayString(localTimezone, time));
@@ -49,7 +55,9 @@ public class DateUtilTest {
System.out.println("EST time :" +
DateUtil.getDateDisplayString(estTimezone, time));
//Test next run time. Expects interval and schedule as arguments
if (args.length == 2) {
- System.out.println("Next run time: " +
DateUtil.getNextRunTime(IntervalType.getIntervalType(args[0]), args[1], "GMT",
time).toString());
+ System.out.println("Next run time: " +
DateUtil.getNextRunTime(IntervalType.getIntervalType(args[0]),
+ args[1], "GMT", time)
+ .toString());
}
}
@@ -130,4 +138,87 @@ public class DateUtilTest {
assertEquals(str, time.toString(), dtParsed.toString());
}
+
+ @Test
+ public void parseDateStringDefaultFormat() {
+ TimeZone gmt = GMT;
+ Date date = DateUtil.parseDateString(gmt, TEST_DATE_GMT);
+ assertEquals(TEST_DATE_GMT, DateUtil.getDateDisplayString(gmt, date));
+ }
+
+ @Test
+ public void parseDateStringInterpretedInRequestedTimezone() {
+ TimeZone est = EST;
+ Date date = DateUtil.parseDateString(est, TEST_DATE_EST);
+ assertEquals(TEST_DATE_GMT, DateUtil.getDateDisplayString(GMT, date));
+ }
+
+ @Test
+ public void parseDateStringCustomFormat() {
+ TimeZone gmt = GMT;
+ Date date = DateUtil.parseDateString(gmt, TEST_DATE_YYMMDD,
DateUtil.YYYYMMDD_FORMAT);
+ assertEquals(TEST_DATE_GMT, DateUtil.getDateDisplayString(gmt, date));
+ }
+
+ @Test(expected = CloudRuntimeException.class)
+ public void parseDateStringInvalidInputThrows() {
+ DateUtil.parseDateString(GMT, "not-a-date");
+ }
+
+ @Test(expected = CloudRuntimeException.class)
+ public void parseDateStringFormatMismatchThrows() {
+ DateUtil.parseDateString(GMT, TEST_DATE_GMT, DateUtil.YYYYMMDD_FORMAT);
+ }
+
+ @Test
+ public void displayDateInTimezoneGmt() {
+ Date date = Date.from(Instant.parse(TEST_DATE_ISO));
+ assertEquals("2023-06-15T10:30:00+0000",
DateUtil.displayDateInTimezone(GMT, date));
+ }
+
+ @Test
+ public void displayDateInTimezoneEst() {
+ Date date = Date.from(Instant.parse(TEST_DATE_ISO));
+ assertEquals("2023-06-15T05:30:00-0500",
DateUtil.displayDateInTimezone(EST, date));
+ }
+
+ @Test
+ public void getDateDisplayStringGmt() {
+ Date date = Date.from(Instant.parse(TEST_DATE_ISO));
+ assertEquals(TEST_DATE_GMT, DateUtil.getDateDisplayString(GMT, date));
+ }
+
+ @Test
+ public void getDateDisplayStringTimezoneShift() {
+ Date date = Date.from(Instant.parse(TEST_DATE_ISO));
+ assertEquals(TEST_DATE_EST, DateUtil.getDateDisplayString(EST, date));
+ }
+
+ @Test
+ public void getDateDisplayStringCustomFormat() {
+ Date date = Date.from(Instant.parse(TEST_DATE_ISO));
+ assertEquals(TEST_DATE_YYMMDD, DateUtil.getDateDisplayString(GMT,
date, DateUtil.YYYYMMDD_FORMAT));
+ }
+
+ @Test
+ public void getDateDisplayStringNullDate() {
+ assertEquals(null, DateUtil.getDateDisplayString(GMT, null));
+ }
+
+ @Test
+ public void displayDateInTimezoneNullDate() {
+ assertEquals(null, DateUtil.displayDateInTimezone(GMT, null));
+ }
+
+ @Test
+ public void getOutputStringNull() {
+ assertEquals("", DateUtil.getOutputString(null));
+ }
+
+ @Test
+ public void getOutputStringNonNull() {
+ Date date = Date.from(Instant.parse(TEST_DATE_ISO));
+ String result = DateUtil.getOutputString(date);
+
assertTrue(result.matches("\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}[+-]\\d{4}"));
+ }
}
