Skip to content

Commit

Permalink
chore: Refactor JdbcTemplate to NamedParameterJdbcTemplate (#10767)
Browse files Browse the repository at this point in the history
  • Loading branch information
@stian-sandvold
stian-sandvold authored Jun 2, 2022
1 parent 48d5038 commit ae68067
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 110 deletions.
188 changes: 89 additions & 99 deletions ...vices/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/events/event/JdbcEventStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@
import static org.hisp.dhis.dxf2.events.trackedentity.store.query.EventQuery.COLUMNS.UPDATED;
import static org.hisp.dhis.dxf2.events.trackedentity.store.query.EventQuery.COLUMNS.UPDATEDCLIENT;
import static org.hisp.dhis.system.util.SqlUtils.castToNumber;
import static org.hisp.dhis.system.util.SqlUtils.escapeSql;
import static org.hisp.dhis.system.util.SqlUtils.lower;
import static org.hisp.dhis.util.DateUtils.getDateAfterAddition;
import static org.hisp.dhis.util.DateUtils.getLongGmtDateString;
Expand All @@ -88,7 +87,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -140,9 +138,10 @@
import org.locationtech.jts.io.WKTReader;
import org.postgresql.util.PGobject;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.core.env.Environment;
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.jdbc.core.namedparam.SqlParameterSource;
import org.springframework.jdbc.support.rowset.SqlRowSet;
import org.springframework.stereotype.Repository;

Expand All @@ -154,6 +153,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.gson.Gson;

Expand Down Expand Up @@ -305,10 +305,8 @@ public class JdbcEventStore implements EventStore
* actual UPDATE statement. This prevents deadlocks when Postgres tries to
* update the same TEI.
*/
private static final String UPDATE_TEI_SQL = "SELECT * FROM trackedentityinstance where uid in (%s) FOR UPDATE %s;"
+ "update trackedentityinstance set lastupdated = %s, lastupdatedby = %s where uid in (%s)";

private static final String NULL = "null";
private static final String UPDATE_TEI_SQL = "SELECT * FROM trackedentityinstance WHERE uid IN (:teiUids) FOR UPDATE SKIP LOCKED;"
+ "UPDATE trackedentityinstance SET lastupdated = :lastUpdated, lastupdatedby = :lastUpdatedBy WHERE uid IN (:teiUids)";

static
{
Expand All @@ -323,10 +321,10 @@ public class JdbcEventStore implements EventStore

UPDATE_EVENT_SQL = "update programstageinstance set " +
UPDATE_COLUMNS.stream()
.map( column -> column + " = ?" )
.map( column -> column + " = :" + column )
.limit( UPDATE_COLUMNS.size() - 1 )
.collect( Collectors.joining( "," ) )
+ " where uid = ?;";
+ " where uid = :uid;";
}

// -------------------------------------------------------------------------
Expand All @@ -342,7 +340,7 @@ public class JdbcEventStore implements EventStore

private final StatementBuilder statementBuilder;

private final JdbcTemplate jdbcTemplate;
private final NamedParameterJdbcTemplate jdbcTemplate;

@Qualifier( "dataValueJsonMapper" )
private final ObjectMapper jsonMapper;
Expand All @@ -351,8 +349,6 @@ public class JdbcEventStore implements EventStore

private final IdentifiableObjectManager manager;

private final Environment env;

private final org.hisp.dhis.dxf2.events.trackedentity.store.EventStore eventStore;

private final SkipLockedProvider skipLockedProvider;
Expand All @@ -376,7 +372,7 @@ public List<Event> getEvents( EventSearchParams params, List<OrganisationUnit> o
final Gson gson = new Gson();

String sql = buildSql( params, organisationUnits, user );
SqlRowSet rowSet = jdbcTemplate.queryForRowSet( sql );
SqlRowSet rowSet = jdbcTemplate.getJdbcTemplate().queryForRowSet( sql );

log.debug( "Event query SQL: " + sql );

Expand Down Expand Up @@ -602,18 +598,23 @@ public List<ProgramStageInstance> updateEvents( List<ProgramStageInstance> progr
{
try
{
jdbcTemplate.batchUpdate( UPDATE_EVENT_SQL, sort( programStageInstances ), programStageInstances.size(),
( ps, programStageInstance ) -> {
try
{
bindEventParamsForUpdate( ps, programStageInstance );
}
catch ( JsonProcessingException | SQLException e )
{
log.warn( "PSI failed to update and will be ignored. PSI UID: " + programStageInstance.getUid(),
programStageInstance.getUid(), e );
}
} );
SqlParameterSource[] parameters = new SqlParameterSource[programStageInstances.size()];

for ( int i = 0; i < programStageInstances.size(); i++ )
{
try
{
parameters[i] = getSqlParameters( programStageInstances.get( i ) );
}
catch ( SQLException | JsonProcessingException e )
{
log.warn(
"PSI failed to update and will be ignored. PSI UID: " + programStageInstances.get( i ).getUid(),
programStageInstances.get( i ).getUid(), e );
}
}

jdbcTemplate.batchUpdate( UPDATE_EVENT_SQL, parameters );
}
catch ( DataAccessException e )
{
Expand All @@ -633,7 +634,7 @@ public List<Map<String, String>> getEventsGrid( EventSearchParams params, List<O

String sql = buildGridSql( params, organisationUnits );

SqlRowSet rowSet = jdbcTemplate.queryForRowSet( sql );
SqlRowSet rowSet = jdbcTemplate.getJdbcTemplate().queryForRowSet( sql );

log.debug( "Event query SQL: " + sql );

Expand Down Expand Up @@ -670,7 +671,7 @@ public List<EventRow> getEventRows( EventSearchParams params, List<OrganisationU

String sql = buildSql( params, organisationUnits, user );

SqlRowSet rowSet = jdbcTemplate.queryForRowSet( sql );
SqlRowSet rowSet = jdbcTemplate.getJdbcTemplate().queryForRowSet( sql );

log.debug( "Event query SQL: " + sql );

Expand Down Expand Up @@ -883,7 +884,7 @@ public int getEventCount( EventSearchParams params, List<OrganisationUnit> organ

log.debug( "Event query count SQL: " + sql );

return jdbcTemplate.queryForObject( sql, Integer.class );
return jdbcTemplate.getJdbcTemplate().queryForObject( sql, Integer.class );
}

private DataValue convertEventDataValueIntoDtoDataValue( EventDataValue eventDataValue )
Expand Down Expand Up @@ -1639,8 +1640,9 @@ private boolean isSuper( User user )
*/
private List<ProgramStageInstance> saveAllEvents( List<ProgramStageInstance> batch )
{
JdbcUtils.batchUpdateWithKeyHolder( jdbcTemplate, INSERT_EVENT_SQL,
new BatchPreparedStatementSetterWithKeyHolder<ProgramStageInstance>( sort( batch ) )

JdbcUtils.batchUpdateWithKeyHolder( jdbcTemplate.getJdbcTemplate(), INSERT_EVENT_SQL,
new BatchPreparedStatementSetterWithKeyHolder<>( sort( batch ) )
{
@Override
protected void setValues( PreparedStatement ps, ProgramStageInstance event )
Expand Down Expand Up @@ -1684,7 +1686,8 @@ protected void setPrimaryKey( Map<String, Object> primaryKey, ProgramStageInstan
Map<String, Long> persisted = jdbcTemplate
.queryForList(
"SELECT uid, programstageinstanceid from programstageinstance where programstageinstanceid in ( "
+ Joiner.on( ";" ).join( eventIds ) + ")" )
+ Joiner.on( ";" ).join( eventIds ) + ")",
Maps.newHashMap() )
.stream().collect(
Collectors.toMap( s -> (String) s.get( "uid" ), s -> (Long) s.get( "programstageinstanceid" ) ) );

Expand All @@ -1708,31 +1711,23 @@ protected void setPrimaryKey( Map<String, Object> primaryKey, ProgramStageInstan
@Override
public void updateTrackedEntityInstances( List<String> teiUids, User user )
{
Optional.ofNullable( teiUids ).filter( s -> !s.isEmpty() )
.ifPresent( teis -> updateTrackedEntityInstances( teis.stream()
.sorted() // make sure the list is sorted, to prevent
// deadlocks
.map( s -> "'" + s + "'" )
.collect( Collectors.joining( ", " ) ), user ) );
}

private void updateTrackedEntityInstances( String teisInCondition, User user )
{
try
if ( teiUids.isEmpty() )
{
Timestamp timestamp = new Timestamp( System.currentTimeMillis() );
return;
}

String sql = String.format( UPDATE_TEI_SQL, teisInCondition, skipLockedProvider.getSkipLocked(),
"'" + timestamp + "'",
user != null ? user.getId() : NULL, teisInCondition );
String sql = UPDATE_TEI_SQL;

jdbcTemplate.execute( sql );
}
catch ( DataAccessException e )
if ( skipLockedProvider.getSkipLocked().isEmpty() )
{
log.error( "An error occurred updating one or more Tracked Entity Instances", e );
throw e;
sql = sql.replace( "SKIP LOCKED", "" );
}

jdbcTemplate.execute( sql, new MapSqlParameterSource()
.addValue( "teiUids", teiUids )
.addValue( "lastUpdated", new Timestamp( System.currentTimeMillis() ) )
.addValue( "lastUpdatedBy", (user != null ? user.getId() : null) ),
PreparedStatement::execute );
}

private void bindEventParamsForInsert( PreparedStatement ps, ProgramStageInstance event )
Expand Down Expand Up @@ -1772,47 +1767,41 @@ private void bindEventParamsForInsert( PreparedStatement ps, ProgramStageInstanc
// @formatter:on
}

private void bindEventParamsForUpdate( PreparedStatement ps, ProgramStageInstance programStageInstance )
private MapSqlParameterSource getSqlParameters( ProgramStageInstance programStageInstance )
throws SQLException,
JsonProcessingException
{

ps.setLong( 1, programStageInstance.getProgramInstance().getId() );
ps.setLong( 2, programStageInstance.getProgramStage().getId() );
ps.setTimestamp( 3, new Timestamp( programStageInstance.getDueDate().getTime() ) );
if ( programStageInstance.getExecutionDate() != null )
{
ps.setTimestamp( 4, new Timestamp( programStageInstance.getExecutionDate().getTime() ) );
}
else
{
ps.setObject( 4, null );
}
ps.setLong( 5, programStageInstance.getOrganisationUnit().getId() );
ps.setString( 6, programStageInstance.getStatus().toString() );
ps.setTimestamp( 7, JdbcEventSupport.toTimestamp( programStageInstance.getCompletedDate() ) );
ps.setTimestamp( 8, JdbcEventSupport.toTimestamp( new Date() ) );
ps.setLong( 9, programStageInstance.getAttributeOptionCombo().getId() );
ps.setString( 10, programStageInstance.getStoredBy() );
ps.setObject( 11, userInfoToJson( programStageInstance.getLastUpdatedByUserInfo(), jsonMapper ) );
ps.setString( 12, programStageInstance.getCompletedBy() );
ps.setBoolean( 13, programStageInstance.isDeleted() );
ps.setString( 14, programStageInstance.getCode() );
ps.setTimestamp( 15, JdbcEventSupport.toTimestamp( programStageInstance.getCreatedAtClient() ) );
ps.setTimestamp( 16, JdbcEventSupport.toTimestamp( programStageInstance.getLastUpdatedAtClient() ) );
ps.setObject( 17, JdbcEventSupport.toGeometry( programStageInstance.getGeometry() ) );

if ( programStageInstance.getAssignedUser() != null )
{
ps.setLong( 18, programStageInstance.getAssignedUser().getId() );
}
else
{
ps.setObject( 18, null );
}

ps.setObject( 19, eventDataValuesToJson( programStageInstance.getEventDataValues(), this.jsonMapper ) );
ps.setString( 20, programStageInstance.getUid() );
return new MapSqlParameterSource()
.addValue( "programInstanceId", programStageInstance.getProgramInstance().getId() )
.addValue( "programstageid", programStageInstance.getProgramStage().getId() )
.addValue( DUE_DATE.getColumnName(), new Timestamp( programStageInstance.getDueDate().getTime() ) )
.addValue( EXECUTION_DATE.getColumnName(),
(programStageInstance.getExecutionDate() != null
? new Timestamp( programStageInstance.getExecutionDate().getTime() )
: null) )
.addValue( "organisationunitid", programStageInstance.getOrganisationUnit().getId() )
.addValue( STATUS.getColumnName(), programStageInstance.getStatus().toString() )
.addValue( COMPLETEDDATE.getColumnName(),
JdbcEventSupport.toTimestamp( programStageInstance.getCompletedDate() ) )
.addValue( UPDATED.getColumnName(), JdbcEventSupport.toTimestamp( new Date() ) )
.addValue( "attributeoptioncomboid", programStageInstance.getAttributeOptionCombo().getId() )
.addValue( STOREDBY.getColumnName(), programStageInstance.getStoredBy() )
.addValue( "lastupdatedbyuserinfo",
userInfoToJson( programStageInstance.getLastUpdatedByUserInfo(), jsonMapper ) )
.addValue( COMPLETEDBY.getColumnName(), programStageInstance.getCompletedBy() )
.addValue( DELETED.getColumnName(), programStageInstance.isDeleted() )
.addValue( "code", programStageInstance.getCode() )
.addValue( CREATEDCLIENT.getColumnName(),
JdbcEventSupport.toTimestamp( programStageInstance.getCreatedAtClient() ) )
.addValue( UPDATEDCLIENT.getColumnName(),
JdbcEventSupport.toTimestamp( programStageInstance.getLastUpdatedAtClient() ) )
.addValue( GEOMETRY.getColumnName(), JdbcEventSupport.toGeometry( programStageInstance.getGeometry() ) )
.addValue( "assigneduserid",
(programStageInstance.getAssignedUser() != null ? programStageInstance.getAssignedUser().getId()
: null) )
.addValue( "eventdatavalues",
eventDataValuesToJson( programStageInstance.getEventDataValues(), jsonMapper ) )
.addValue( UID.getColumnName(), programStageInstance.getUid() );
}

private boolean userHasAccess( SqlRowSet rowSet )
Expand Down Expand Up @@ -1882,13 +1871,14 @@ private void populateCache( IdScheme idScheme, List<Collection<DataValue>> dataV
{
if ( !deUids.isEmpty() )
{
String dataElementsUidsSqlString = getQuotedCommaDelimitedString( deUids );

String deSql = "select de.uid, de.attributevalues #>> '{" + escapeSql( idScheme.getAttribute() )
+ ", value}' as value from dataelement de where de.uid in (" + dataElementsUidsSqlString + ") "
+ "and de.attributevalues ? '" + escapeSql( idScheme.getAttribute() ) + "'";
String sql = "select de.uid, de.attributevalues #>> :attributeValuePath::text[] as value "
+ "from dataelement de where de.uid in (:dataElementUids) "
+ "and de.attributevalues ?? :attributeUid";

SqlRowSet deRowSet = jdbcTemplate.queryForRowSet( deSql );
SqlRowSet deRowSet = jdbcTemplate.queryForRowSet( sql, new MapSqlParameterSource()
.addValue( "attributeValuePath", "{" + idScheme.getAttribute() + ",value}" )
.addValue( "attributeUid", idScheme.getAttribute() )
.addValue( "dataElementUids", deUids ) );

while ( deRowSet.next() )
{
Expand All @@ -1903,11 +1893,11 @@ public void delete( final List<Event> events )
{
if ( isNotEmpty( events ) )
{
final List<String> psiUids = events.stream().map( event -> "'" + event.getEvent() + "'" )
final List<String> psiUids = events.stream().map( Event::getEvent )
.collect( toList() );
final String uids = Joiner.on( "," ).join( psiUids );

jdbcTemplate.execute( "UPDATE programstageinstance SET deleted = true where uid in ( " + uids + ")" );
jdbcTemplate.update( "UPDATE programstageinstance SET deleted = true where uid in (:uids)",
new MapSqlParameterSource().addValue( "uids", psiUids ) );
}
}

Expand Down
3 changes: 2 additions & 1 deletion ...main/java/org/hisp/dhis/dxf2/events/event/persistence/DefaultEventPersistenceService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ private void updateTeis( final WorkContext context, final List<Event> events )
.map( e -> context.getTrackedEntityInstance( e.getUid() ) )
.filter( Optional::isPresent )
.map( o -> o.get().getUid() )
.distinct().collect( Collectors.toList() );
.distinct()
.collect( Collectors.toList() );

jdbcEventStore.updateTrackedEntityInstances( distinctTeiList, context.getImportOptions().getUser() );
}
Expand Down
20 changes: 17 additions & 3 deletions ...s-services/dhis-service-dxf2/src/test/java/org/hisp/dhis/dxf2/events/EventImportTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.sql.Timestamp;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
Expand Down Expand Up @@ -89,6 +90,7 @@
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.jdbc.core.JdbcTemplate;

import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
Expand Down Expand Up @@ -131,6 +133,9 @@ class EventImportTest extends TransactionalIntegrationTest
@Autowired
private SessionFactory sessionFactory;

@Autowired
JdbcTemplate jdbcTemplate;

private TrackedEntityInstance trackedEntityInstanceMaleA;

private OrganisationUnit organisationUnitA;
Expand Down Expand Up @@ -339,10 +344,19 @@ void testAddEventOnProgramWithRegistration()
ImportSummaries importSummaries = eventService.addEventsJson( is, null );
assertEquals( ImportStatus.SUCCESS, importSummaries.getStatus() );
cleanSession();

// We use JDBC to get the timestamp, since it's stored using JDBC not
// hibernate.
String lastupdateDateNew = DateUtils.getIso8601NoTz( this.jdbcTemplate.queryForObject(
"SELECT lastupdated FROM trackedentityinstance WHERE uid IN ('"
+ trackedEntityInstanceMaleA.getTrackedEntityInstance() + "')",
Timestamp.class ) );

assertTrue( simpleDateFormat
.parse( trackedEntityInstanceService
.getTrackedEntityInstance( trackedEntityInstanceMaleA.getTrackedEntityInstance() ).getLastUpdated() )
.getTime() > simpleDateFormat.parse( lastupdateDateBefore ).getTime() );
.parse( lastupdateDateNew )
.getTime() > simpleDateFormat
.parse( lastupdateDateBefore )
.getTime() );
}

@Test
Expand Down
Loading

0 comments on commit ae68067

Please sign in to comment.