spring-projects / spring-batch

Spring Batch is a framework for writing batch applications using Java and Spring
http://projects.spring.io/spring-batch/
Apache License 2.0
2.72k stars 2.35k forks source link

JdbcJobExecutionDao throws NullPointerException #3990

Open adrianriley opened 3 years ago

adrianriley commented 3 years ago

Bug description BATCH_JOB_EXECUTION.STATUS may be null. If it is, a NullPointerException is thrown by JdbcJobExecutionDao.JobExecutionRowMapper.mapRow, See this line:

jobExecution.setStatus(BatchStatus.valueOf(rs.getString(4)));

Environment spring-batch-core 4.2.1.RELEASE. But the bug still appears to be there in 5.0.0-SNAPSHOT

Steps to reproduce Actually happened calling JdbcJobExecutionDao.findRunningJobExecutions(), but you can see it on the UI. Pick an instance. Update BATCH_JOB_EXECUTION.STATUS to null. Try to view it on .../dashboard/#/tasks-jobs/task-executions/nnnn

Expected behavior I'd expect status to be blank. Not a NullPointerException.

Minimal Complete Reproducible example

package uk.co.closemf.collection.support.event.batch;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.batch.core.repository.dao.JdbcJobExecutionDao;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowCallbackHandler;

import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Timestamp;

import static org.assertj.core.api.Assertions.assertThatCode;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
public class JdbcJobExecutonDaoTest {
    @InjectMocks
    private JdbcJobExecutionDao jdbcJobExecutionDao;

    @Mock
    private JdbcTemplate jdbcTemplate;

    @Test
    void mapRow() throws SQLException {
        ResultSet rs = mock(ResultSet.class);
        when(rs.next()).thenReturn(true, false);
        when(rs.getLong(1)).thenReturn(1L);
        when(rs.getTimestamp(anyInt())).thenReturn(new Timestamp(System.currentTimeMillis()));
        when(rs.getString(anyInt())).thenAnswer(invocation -> {
            int i = invocation.getArgument(0, Integer.class);
            switch (i) {
                case 3:
                    return "STRING";
                case 4:
                    return null;
                default:
                    return "";
            }
        });
        when(rs.getInt(9)).thenReturn(1);
        lenient().doAnswer(invocation -> {
            RowCallbackHandler rch = invocation.getArgument(2, RowCallbackHandler.class);
            rch.processRow(rs);
            return null;
        }).when(jdbcTemplate)
                .query(anyString(), any(Object[].class), any(RowCallbackHandler.class));

        ;

        assertThatCode(() -> jdbcJobExecutionDao.findRunningJobExecutions("job"))
                .doesNotThrowAnyException();
    }
}
fmbenhassine commented 3 years ago

BATCH_JOB_EXECUTION.STATUS should not be null. It starts with STARTING and is updated to other non null values during the job execution. Unless updated manually to null in the database (which is not supposed to be done), I don't see where it could be set to null in the code. So before fixing the bug in any UI, I would like to see the circumstances under which this column ends up being null. Can you please share more details about how/when this happens?

adrianriley commented 3 years ago

Sorry, I can't see how it happened either. I've seen one instance from some months ago, but it's only just come to light by implementing use of findRunningJobExecutions(). Logging of the execution which created the row in the SCDF logs is minimal, so my guess is that it was created by running the application outside of the SCDF server, using the same DB. As you say, the initial creation of the row should have status STARTING and I don't see how it can ever be set to null using code in the spring batch libs. So maybe it's a non-issue. But the DB column is nullable so defensively perhaps the code shouldn't assume values are not null.