Skip to content

Commit fee2056

Browse files
sudo87sudo87
committed
Improve date/timestamp handling performance in GenericDaoBase and DateUtil
1 parent 549daae commit fee2056

7 files changed

Lines changed: 338 additions & 77 deletions

File tree

‎engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public class BackupVO implements Backup {
6060
private String backupType;
6161

6262
@Column(name = "date")
63-
@Temporal(value = TemporalType.DATE)
63+
@Temporal(value = TemporalType.TIMESTAMP)
6464
private Date date;
6565

6666
@Column(name = GenericDao.REMOVED_COLUMN)

‎framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java‎

Lines changed: 52 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@
3434
import java.sql.ResultSetMetaData;
3535
import java.sql.SQLException;
3636
import java.sql.Statement;
37+
import java.sql.Time;
38+
import java.sql.Timestamp;
39+
import java.sql.Types;
3740
import java.util.ArrayList;
3841
import java.util.Arrays;
3942
import java.util.Calendar;
@@ -67,7 +70,6 @@
6770
import org.apache.commons.lang3.exception.ExceptionUtils;
6871

6972
import com.amazonaws.util.CollectionUtils;
70-
import com.cloud.utils.DateUtil;
7173
import com.cloud.utils.NumbersUtil;
7274
import com.cloud.utils.Pair;
7375
import com.cloud.utils.Ternary;
@@ -129,6 +131,28 @@ public abstract class GenericDaoBase<T, ID extends Serializable> extends Compone
129131

130132
protected final static TimeZone s_gmtTimeZone = TimeZone.getTimeZone("GMT");
131133

134+
/**
135+
* Returns a fresh GMT {@link Calendar} for a single JDBC get/set timestamp call. Calendar is
136+
* mutable and JDBC drivers may mutate the instance passed to them, so a new one is used per call
137+
* rather than sharing a single instance across concurrent DAO operations.
138+
*/
139+
protected static Calendar gmtCalendar() {
140+
return Calendar.getInstance(s_gmtTimeZone);
141+
}
142+
143+
/**
144+
* Returns the SQL type ({@link Types}) matching the temporal flag of the given attribute, so a
145+
* null date/time/timestamp column is bound with the correct type instead of always TIMESTAMP.
146+
*/
147+
protected static int temporalSqlType(Attribute attr) {
148+
if (attr.is(Attribute.Flag.Date)) {
149+
return Types.DATE;
150+
} else if (attr.is(Attribute.Flag.Time)) {
151+
return Types.TIME;
152+
}
153+
return Types.TIMESTAMP;
154+
}
155+
132156
protected final static Map<Class<?>, GenericDao<?, ? extends Serializable>> s_daoMaps = new ConcurrentHashMap<Class<?>, GenericDao<?, ? extends Serializable>>(71);
133157
private final ConversionSupport _conversionSupport;
134158

@@ -598,20 +622,16 @@ protected void setField(Object entity, Field field, ResultSet rs, int index) thr
598622
field.set(entity, rs.getInt(index));
599623
}
600624
} else if (type == Date.class) {
601-
final Object data = rs.getDate(index);
602-
if (data == null) {
603-
field.set(entity, null);
604-
return;
605-
}
606-
field.set(entity, DateUtil.parseDateString(s_gmtTimeZone, rs.getString(index)));
625+
final Timestamp ts = rs.getTimestamp(index, gmtCalendar());
626+
field.set(entity, ts == null ? null : new Date(ts.getTime()));
607627
} else if (type == Calendar.class) {
608-
final Object data = rs.getDate(index);
628+
final Timestamp data = rs.getTimestamp(index, gmtCalendar());
609629
if (data == null) {
610630
field.set(entity, null);
611631
return;
612632
}
613-
final Calendar cal = Calendar.getInstance();
614-
cal.setTime(DateUtil.parseDateString(s_gmtTimeZone, rs.getString(index)));
633+
final Calendar cal = Calendar.getInstance(s_gmtTimeZone);
634+
cal.setTime(data);
615635
field.set(entity, cal);
616636
} else if (type == boolean.class) {
617637
field.setBoolean(entity, rs.getBoolean(index));
@@ -732,11 +752,11 @@ protected static <M> M getObject(Class<M> type, ResultSet rs, int index) throws
732752
return (M) (Long) rs.getLong(index);
733753
}
734754
} else if (type == Date.class) {
735-
final Object data = rs.getDate(index);
736-
if (data == null) {
755+
final Timestamp ts = rs.getTimestamp(index, gmtCalendar());
756+
if (ts == null) {
737757
return null;
738758
} else {
739-
return (M)DateUtil.parseDateString(s_gmtTimeZone, rs.getString(index));
759+
return (M) new Date(ts.getTime());
740760
}
741761
} else if (type == short.class) {
742762
return (M) (Short) rs.getShort(index);
@@ -779,12 +799,12 @@ protected static <M> M getObject(Class<M> type, ResultSet rs, int index) throws
779799
return (M) (Byte) rs.getByte(index);
780800
}
781801
} else if (type == Calendar.class) {
782-
final Object data = rs.getDate(index);
802+
final Timestamp data = rs.getTimestamp(index, gmtCalendar());
783803
if (data == null) {
784804
return null;
785805
} else {
786-
final Calendar cal = Calendar.getInstance();
787-
cal.setTime(DateUtil.parseDateString(s_gmtTimeZone, rs.getString(index)));
806+
final Calendar cal = Calendar.getInstance(s_gmtTimeZone);
807+
cal.setTime(data);
788808
return (M)cal;
789809
}
790810
} else if (type == byte[].class) {
@@ -1696,7 +1716,12 @@ protected void insertElementCollection(T entity, Attribute idAttribute, ID id, M
16961716
while (en.hasMoreElements()) {
16971717
pstmt = txn.prepareAutoCloseStatement(ec.insertSql);
16981718
if (ec.targetClass == Date.class) {
1699-
pstmt.setString(1, DateUtil.getDateDisplayString(s_gmtTimeZone, (Date)en.nextElement()));
1719+
Date d = (Date) en.nextElement();
1720+
if (d == null) {
1721+
pstmt.setNull(1, Types.TIMESTAMP);
1722+
} else {
1723+
pstmt.setTimestamp(1, new Timestamp(d.getTime()), gmtCalendar());
1724+
}
17001725
} else {
17011726
pstmt.setObject(1, en.nextElement());
17021727
}
@@ -1800,28 +1825,28 @@ protected void prepareAttribute(final int j, final PreparedStatement pstmt, fina
18001825
} else if (attr.field.getType() == Date.class) {
18011826
final Date date = (Date)value;
18021827
if (date == null || date.equals(DATE_TO_NULL)) {
1803-
pstmt.setObject(j, null);
1828+
pstmt.setNull(j, temporalSqlType(attr));
18041829
return;
18051830
}
18061831
if (attr.is(Attribute.Flag.Date)) {
1807-
pstmt.setString(j, DateUtil.getDateDisplayString(s_gmtTimeZone, date));
1832+
pstmt.setDate(j, new java.sql.Date(date.getTime()), gmtCalendar());
18081833
} else if (attr.is(Attribute.Flag.TimeStamp)) {
1809-
pstmt.setString(j, DateUtil.getDateDisplayString(s_gmtTimeZone, date));
1834+
pstmt.setTimestamp(j, new Timestamp(date.getTime()), gmtCalendar());
18101835
} else if (attr.is(Attribute.Flag.Time)) {
1811-
pstmt.setString(j, DateUtil.getDateDisplayString(s_gmtTimeZone, date));
1836+
pstmt.setTime(j, new java.sql.Time(date.getTime()), gmtCalendar());
18121837
}
18131838
} else if (attr.field.getType() == Calendar.class) {
18141839
final Calendar cal = (Calendar)value;
18151840
if (cal == null) {
1816-
pstmt.setObject(j, null);
1841+
pstmt.setNull(j, temporalSqlType(attr));
18171842
return;
18181843
}
18191844
if (attr.is(Attribute.Flag.Date)) {
1820-
pstmt.setString(j, DateUtil.getDateDisplayString(s_gmtTimeZone, cal.getTime()));
1845+
pstmt.setDate(j, new java.sql.Date(cal.getTimeInMillis()), gmtCalendar());
18211846
} else if (attr.is(Attribute.Flag.TimeStamp)) {
1822-
pstmt.setString(j, DateUtil.getDateDisplayString(s_gmtTimeZone, cal.getTime()));
1847+
pstmt.setTimestamp(j, new Timestamp(cal.getTimeInMillis()), gmtCalendar());
18231848
} else if (attr.is(Attribute.Flag.Time)) {
1824-
pstmt.setString(j, DateUtil.getDateDisplayString(s_gmtTimeZone, cal.getTime()));
1849+
pstmt.setTime(j, new Time(cal.getTimeInMillis()), gmtCalendar());
18251850
}
18261851
} else if (attr.field.getType().isEnum()) {
18271852
final Enumerated enumerated = attr.field.getAnnotation(Enumerated.class);
@@ -1955,7 +1980,8 @@ protected void loadCollection(T entity, Attribute attr) {
19551980
}
19561981
} else if (ec.targetClass == Date.class) {
19571982
while (rs.next()) {
1958-
lst.add(DateUtil.parseDateString(s_gmtTimeZone, rs.getString(1)));
1983+
final Timestamp ts = rs.getTimestamp(1, gmtCalendar());
1984+
lst.add(ts == null ? null : new Date(ts.getTime()));
19591985
}
19601986
} else if (ec.targetClass == Boolean.class) {
19611987
while (rs.next()) {

‎framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,14 @@
1818

1919
import java.sql.ResultSet;
2020
import java.sql.SQLException;
21+
import java.sql.Timestamp;
22+
import java.sql.Types;
2123
import java.util.ArrayList;
24+
import java.util.Calendar;
2225
import java.util.Collection;
26+
import java.util.Date;
2327
import java.util.List;
28+
import java.util.TimeZone;
2429

2530
import org.junit.Assert;
2631
import org.junit.Before;
@@ -331,4 +336,74 @@ public void testLockOneRandomRowReturnsFirstElement() {
331336
Assert.assertNotNull(result);
332337
Assert.assertEquals(expectedResult, result);
333338
}
339+
340+
@Test
341+
public void gmtCalendarUsesGmtTimeZone() {
342+
Calendar calendar = GenericDaoBase.gmtCalendar();
343+
344+
Assert.assertEquals(TimeZone.getTimeZone("GMT"), calendar.getTimeZone());
345+
}
346+
347+
@Test
348+
public void gmtCalendarReturnsFreshInstancePerCall() {
349+
Assert.assertNotSame(GenericDaoBase.gmtCalendar(), GenericDaoBase.gmtCalendar());
350+
}
351+
352+
@Test
353+
public void temporalSqlTypeDate() {
354+
Attribute attr = new Attribute("table", "column");
355+
attr.flags = Attribute.Flag.Date.setTrue(attr.flags);
356+
357+
Assert.assertEquals(Types.DATE, GenericDaoBase.temporalSqlType(attr));
358+
}
359+
360+
@Test
361+
public void temporalSqlTypeTime() {
362+
Attribute attr = new Attribute("table", "column");
363+
attr.flags = Attribute.Flag.Time.setTrue(attr.flags);
364+
365+
Assert.assertEquals(Types.TIME, GenericDaoBase.temporalSqlType(attr));
366+
}
367+
368+
@Test
369+
public void temporalSqlTypeDefaultsToTimestamp() {
370+
Attribute attr = new Attribute("table", "column");
371+
attr.flags = Attribute.Flag.TimeStamp.setTrue(attr.flags);
372+
373+
Assert.assertEquals(Types.TIMESTAMP, GenericDaoBase.temporalSqlType(attr));
374+
}
375+
376+
@Test
377+
public void getObjectDateReadsViaGmtTimestamp() throws SQLException {
378+
Timestamp ts = new Timestamp(1_700_000_000_000L);
379+
Mockito.when(resultSet.getTimestamp(Mockito.eq(2), Mockito.any(Calendar.class))).thenReturn(ts);
380+
381+
Date result = GenericDaoBase.getObject(Date.class, resultSet, 2);
382+
383+
Assert.assertEquals(ts.getTime(), result.getTime());
384+
}
385+
386+
@Test
387+
public void getObjectDateNullTimestampReturnsNull() throws SQLException {
388+
Mockito.when(resultSet.getTimestamp(Mockito.eq(3), Mockito.any(Calendar.class))).thenReturn(null);
389+
390+
Assert.assertNull(GenericDaoBase.getObject(Date.class, resultSet, 3));
391+
}
392+
393+
@Test
394+
public void getObjectCalendarReadsViaGmtTimestamp() throws SQLException {
395+
Timestamp ts = new Timestamp(1_700_000_000_000L);
396+
Mockito.when(resultSet.getTimestamp(Mockito.eq(4), Mockito.any(Calendar.class))).thenReturn(ts);
397+
398+
Calendar result = GenericDaoBase.getObject(Calendar.class, resultSet, 4);
399+
400+
Assert.assertEquals(ts.getTime(), result.getTimeInMillis());
401+
}
402+
403+
@Test
404+
public void getObjectCalendarNullTimestampReturnsNull() throws SQLException {
405+
Mockito.when(resultSet.getTimestamp(Mockito.eq(5), Mockito.any(Calendar.class))).thenReturn(null);
406+
407+
Assert.assertNull(GenericDaoBase.getObject(Calendar.class, resultSet, 5));
408+
}
334409
}

‎server/src/main/java/com/cloud/api/ApiServer.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@
192192

193193
import static com.cloud.user.AccountManagerImpl.apiKeyAccess;
194194
import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled;
195+
import static org.apache.commons.lang3.StringUtils.deleteWhitespace;
195196

196197
@Component
197198
public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiServerService, Configurable {
@@ -1372,19 +1373,17 @@ private void checkCommandAvailable(final User user, final String commandName, fi
13721373
throw new PermissionDeniedException("User is null for role based API access check for command" + commandName);
13731374
}
13741375

1375-
final Account account = accountMgr.getAccount(user.getAccountId());
1376-
final String accessAllowedCidrs = ApiServiceConfiguration.ApiAllowedSourceCidrList.valueIn(account.getId()).replaceAll("\\s","");
13771376
final Boolean apiSourceCidrChecksEnabled = ApiServiceConfiguration.ApiSourceCidrChecksEnabled.value();
1378-
13791377
if (apiSourceCidrChecksEnabled) {
1378+
final Account account = accountMgr.getAccount(user.getAccountId());
1379+
final String accessAllowedCidrs = deleteWhitespace(ApiServiceConfiguration.ApiAllowedSourceCidrList.valueIn(account.getId()));
13801380
logger.debug("CIDRs from which account '" + account.toString() + "' is allowed to perform API calls: " + accessAllowedCidrs);
13811381
if (!NetUtils.isIpInCidrList(remoteAddress, accessAllowedCidrs.split(","))) {
13821382
logger.warn("Request by account '" + account.toString() + "' was denied since " + remoteAddress + " does not match " + accessAllowedCidrs);
13831383
throw new OriginDeniedException("Calls from disallowed origin", account, remoteAddress);
13841384
}
13851385
}
13861386

1387-
13881387
for (final APIChecker apiChecker : apiAccessCheckers) {
13891388
apiChecker.checkAccess(user, commandName);
13901389
}

0 commit comments

Comments
 (0)