Skip to content

Prevent NullPointerException on GenericDaoBase #4268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yadvr merged 2 commits into from
Aug 17, 2020
Merged

Prevent NullPointerException on GenericDaoBase #4268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd2abd1
Prevent NullPointerException on GenericDaoBase
GabrielBrascher Aug 15, 2020
576021b
Fix checkstyle: remove unused import
GabrielBrascher Aug 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ public abstract class GenericDaoBase<T, ID extends Serializable> extends Compone
protected static final String SELECT_LAST_INSERT_ID_SQL = "SELECT LAST_INSERT_ID()";
public static final Date DATE_TO_NULL = new Date(Long.MIN_VALUE);

private static final String INTEGRITY_CONSTRAINT_VIOLATION = "23000";
private static final int DUPLICATE_ENTRY_ERRO_CODE = 1062;

protected static final SequenceFetcher s_seqFetcher = SequenceFetcher.getInstance();

public static <J> GenericDao<? extends J, ? extends Serializable> getDao(Class<J> entityType) {
Expand Down Expand Up @@ -850,13 +853,23 @@ public int update(UpdateBuilder ub, final SearchCriteria<?> sc, Integer rows) {
ub.clear();
return result;
} catch (final SQLException e) {
if (e.getSQLState().equals("23000") && e.getErrorCode() == 1062) {
throw new EntityExistsException("Entity already exists ", e);
}
handleEntityExistsException(e);
throw new CloudRuntimeException("DB Exception on: " + pstmt, e);
}
}

/**
* If the SQLException.getSQLState is of 23000 (Integrity Constraint Violation), and the Error Code is 1062 (Duplicate Entry), throws EntityExistsException.
* @throws EntityExistsException
*/
protected static void handleEntityExistsException(SQLException e) throws EntityExistsException {
boolean isIntegrityConstantViolation = INTEGRITY_CONSTRAINT_VIOLATION.equals(e.getSQLState());
Copy link
Member

@yadvr yadvr Aug 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this pattern to check contant.equals(potentially-nullable-obj)

boolean isErrorCodeOfDuplicateEntry = e.getErrorCode() == DUPLICATE_ENTRY_ERRO_CODE;
if (isIntegrityConstantViolation && isErrorCodeOfDuplicateEntry) {
throw new EntityExistsException("Entity already exists ", e);
}
}

@DB()
protected Attribute findAttributeByFieldName(String name) {
return _allAttributes.get(name);
Expand Down Expand Up @@ -1450,11 +1463,8 @@ public T persist(final T entity) {
}
txn.commit();
} catch (final SQLException e) {
if (e.getSQLState().equals("23000") && e.getErrorCode() == 1062) {
throw new EntityExistsException("Entity already exists: ", e);
} else {
throw new CloudRuntimeException("DB Exception on: " + pstmt, e);
}
handleEntityExistsException(e);
throw new CloudRuntimeException("DB Exception on: " + pstmt, e);
} catch (IllegalArgumentException e) {
throw new CloudRuntimeException("Problem with getting the ec attribute ", e);
} catch (IllegalAccessException e) {
Expand Down
88 changes: 61 additions & 27 deletions framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,132 +20,166 @@
import java.sql.SQLException;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.junit.MockitoJUnitRunner;
Copy link
Member Author

@GabrielBrascher GabrielBrascher Aug 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous MockitoJUnitRunner got Deprecated; changed to a non-deprecated one.


import javax.persistence.EntityExistsException;

@RunWith(MockitoJUnitRunner.class)
public class GenericDaoBaseTest {
@Mock
ResultSet resultSet;
@Mock
SQLException mockedSQLException;

@Test
public void getObjectBoolean() throws SQLException {
private static final String INTEGRITY_CONSTRAINT_VIOLATION = "23000";
private static final int DUPLICATE_ENTRY_ERRO_CODE = 1062;

@Before
public void prepareTests() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(false);
Mockito.when(resultSet.getBoolean(1)).thenReturn(false);
Mockito.when(resultSet.getObject(1)).thenReturn((short) 1);
Copy link
Member Author

@GabrielBrascher GabrielBrascher Aug 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite all tests pass, the Process finished with exit code 255 due to unnecessary stubbings.
I fixed this issue by extracting all into a @Before method and tests are looking good.

The output of tests before this PR:

Process finished with exit code 255

org.mockito.exceptions.misusing.UnnecessaryStubbingException: 
Unnecessary stubbings detected in test class: GenericDaoBaseTest
Clean & maintainable test code requires zero unnecessary code.
Following stubbings are unnecessary (click to navigate to relevant line of code):
  1. -> at com.cloud.utils.db.GenericDaoBaseTest.getObjectPrimitiveByte(GenericDaoBaseTest.java:144)
  2. -> at com.cloud.utils.db.GenericDaoBaseTest.getObjectPrimitiveLong(GenericDaoBaseTest.java:117)
  3. -> at com.cloud.utils.db.GenericDaoBaseTest.getObjectPrimitiveInt(GenericDaoBaseTest.java:126)
  4. -> at com.cloud.utils.db.GenericDaoBaseTest.getObjectPrimitiveDouble(GenericDaoBaseTest.java:90)
  5. -> at com.cloud.utils.db.GenericDaoBaseTest.getObjectPrimitiveFloat(GenericDaoBaseTest.java:81)
  6. -> at com.cloud.utils.db.GenericDaoBaseTest.getObjectPrimitiveShort(GenericDaoBaseTest.java:54)
  7. -> at com.cloud.utils.db.GenericDaoBaseTest.getObjectPrimitiveBoolean(GenericDaoBaseTest.java:45)
Please remove unnecessary stubbings or use 'lenient' strictness. More info: javadoc for UnnecessaryStubbingException class.

	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:49)
	at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:163)
	at org.mockito.runners.MockitoJUnitRunner.run(MockitoJUnitRunner.java:54)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:220)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:53)

Mockito.when(resultSet.getShort(1)).thenReturn((short) 1);
Mockito.when(resultSet.getObject(1)).thenReturn(0.1f);
Mockito.when(resultSet.getFloat(1)).thenReturn(0.1f);
Mockito.when(resultSet.getObject(1)).thenReturn(0.1d);
Mockito.when(resultSet.getDouble(1)).thenReturn(0.1d);
Mockito.when(resultSet.getObject(1)).thenReturn(1l);
Mockito.when(resultSet.getLong(1)).thenReturn(1l);
Mockito.when(resultSet.getInt(1)).thenReturn(1);
Mockito.when(resultSet.getObject(1)).thenReturn((byte) 1);
Mockito.when(resultSet.getByte(1)).thenReturn((byte) 1);
}

@Test
public void getObjectBoolean() throws SQLException {
Assert.assertFalse(GenericDaoBase
.getObject(Boolean.class, resultSet, 1));
Mockito.verify(resultSet).getBoolean(1);
}

@Test
public void getObjectPrimitiveBoolean() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(false);
Mockito.when(resultSet.getBoolean(1)).thenReturn(false);
Assert.assertFalse(GenericDaoBase
.getObject(boolean.class, resultSet, 1));
Mockito.verify(resultSet).getBoolean(1);
}

@Test
public void getObjectPrimitiveShort() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn((short) 1);
Mockito.when(resultSet.getShort(1)).thenReturn((short) 1);
Assert.assertEquals(Short.valueOf((short) 1),
GenericDaoBase.getObject(short.class, resultSet, 1));
Mockito.verify(resultSet).getShort(1);
}

@Test
public void getObjectShort() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn((short) 1);
Mockito.when(resultSet.getShort(1)).thenReturn((short) 1);
Assert.assertEquals(Short.valueOf((short) 1),
GenericDaoBase.getObject(Short.class, resultSet, 1));
Mockito.verify(resultSet).getShort(1);
}

@Test
public void getObjectFloat() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(0.1f);
Mockito.when(resultSet.getFloat(1)).thenReturn(0.1f);
Assert.assertEquals(0.1f,
GenericDaoBase.getObject(Float.class, resultSet, 1), 0.1);
Mockito.verify(resultSet).getFloat(1);
}

@Test
public void getObjectPrimitiveFloat() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(0.1f);
Mockito.when(resultSet.getFloat(1)).thenReturn(0.1f);
Assert.assertEquals(0.1f,
GenericDaoBase.getObject(float.class, resultSet, 1), 0.1);
Mockito.verify(resultSet).getFloat(1);
}

@Test
public void getObjectPrimitiveDouble() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(0.1d);
Mockito.when(resultSet.getDouble(1)).thenReturn(0.1d);
Assert.assertEquals(0.1d,
GenericDaoBase.getObject(double.class, resultSet, 1), 0.1);
Mockito.verify(resultSet).getDouble(1);
}

@Test
public void getObjectDouble() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(0.1d);
Mockito.when(resultSet.getDouble(1)).thenReturn(0.1d);
Assert.assertEquals(0.1d,
GenericDaoBase.getObject(Double.class, resultSet, 1), 0.1);
Mockito.verify(resultSet).getDouble(1);
}

@Test
public void getObjectLong() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(1l);
Mockito.when(resultSet.getLong(1)).thenReturn(1l);
Assert.assertEquals((Long) 1l,
GenericDaoBase.getObject(Long.class, resultSet, 1));
Mockito.verify(resultSet).getLong(1);
}

@Test
public void getObjectPrimitiveLong() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(1l);
Mockito.when(resultSet.getLong(1)).thenReturn(1l);
Assert.assertEquals((Long) 1l,
GenericDaoBase.getObject(long.class, resultSet, 1));
Mockito.verify(resultSet).getLong(1);
}

@Test
public void getObjectPrimitiveInt() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(1l);
Mockito.when(resultSet.getInt(1)).thenReturn(1);
Assert.assertEquals((Integer) 1,
GenericDaoBase.getObject(int.class, resultSet, 1));
Mockito.verify(resultSet).getInt(1);
}

@Test
public void getObjectInteger() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn(1l);
Mockito.when(resultSet.getInt(1)).thenReturn(1);
Assert.assertEquals((Integer) 1,
GenericDaoBase.getObject(Integer.class, resultSet, 1));
Mockito.verify(resultSet).getInt(1);
}

@Test
public void getObjectPrimitiveByte() throws SQLException {
Mockito.when(resultSet.getObject(1)).thenReturn((byte) 1);
Mockito.when(resultSet.getByte(1)).thenReturn((byte) 1);
Assert.assertTrue((byte) 1 == GenericDaoBase.getObject(byte.class,
resultSet, 1));
Mockito.verify(resultSet).getByte(1);
}

@Test
public void handleEntityExistsExceptionTestNoMatchForEntityExists() {
Mockito.when(mockedSQLException.getErrorCode()).thenReturn(123);
Mockito.when(mockedSQLException.getSQLState()).thenReturn("123");
GenericDaoBase.handleEntityExistsException(mockedSQLException);
}

@Test
public void handleEntityExistsExceptionTestIntegrityConstraint() {
Mockito.when(mockedSQLException.getErrorCode()).thenReturn(123);
Mockito.when(mockedSQLException.getSQLState()).thenReturn(INTEGRITY_CONSTRAINT_VIOLATION);
GenericDaoBase.handleEntityExistsException(mockedSQLException);
}

@Test
public void handleEntityExistsExceptionTestIntegrityConstraintNull() {
Mockito.when(mockedSQLException.getErrorCode()).thenReturn(123);
Mockito.when(mockedSQLException.getSQLState()).thenReturn(null);
GenericDaoBase.handleEntityExistsException(mockedSQLException);
}

@Test
public void handleEntityExistsExceptionTestDuplicateEntryErrorCode() {
Mockito.when(mockedSQLException.getErrorCode()).thenReturn(DUPLICATE_ENTRY_ERRO_CODE);
Mockito.when(mockedSQLException.getSQLState()).thenReturn("123");
GenericDaoBase.handleEntityExistsException(mockedSQLException);
}

@Test(expected = EntityExistsException.class)
public void handleEntityExistsExceptionTestExpectEntityExistsException() {
Mockito.when(mockedSQLException.getErrorCode()).thenReturn(DUPLICATE_ENTRY_ERRO_CODE);
Mockito.when(mockedSQLException.getSQLState()).thenReturn(INTEGRITY_CONSTRAINT_VIOLATION);
GenericDaoBase.handleEntityExistsException(mockedSQLException);
}

}