Browse Source

DATAMONGO-1276 - Fixed potential NullPointerExceptions in MongoTemplate.

Triggering data access exception translation could lead to NullPointerException in cases where PersistenceExceptionTranslator returned null because the original exception couldn't be translated and the result was directly used from a throw clause.

This is now fixed by consistently the potentiallyConvertRuntimeException(…) method, which was made static to be able to refer to it from nested static classes.

Refactored Scanner usage to actually close the Scanner instance to prevent a resource leak.
pull/663/head
Oliver Gierke 10 years ago
parent
commit
9968b752e7
  1. 63
      spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java
  2. 83
      spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/CloseableIterableCursorAdapterUnitTests.java

63
spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java

@ -341,7 +341,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -341,7 +341,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
ReadDbObjectCallback<T> readCallback = new ReadDbObjectCallback<T>(mongoConverter, entityType, collection
.getName());
return new CloseableIterableCusorAdapter<T>(cursorPreparer.prepare(cursor), exceptionTranslator, readCallback);
return new CloseableIterableCursorAdapter<T>(cursorPreparer.prepare(cursor), exceptionTranslator, readCallback);
}
});
}
@ -445,7 +445,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -445,7 +445,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
DB db = this.getDb();
return action.doInDB(db);
} catch (RuntimeException e) {
throw potentiallyConvertRuntimeException(e);
throw potentiallyConvertRuntimeException(e, exceptionTranslator);
}
}
@ -461,7 +461,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -461,7 +461,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
DBCollection collection = getAndPrepareCollection(getDb(), collectionName);
return callback.doInCollection(collection);
} catch (RuntimeException e) {
throw potentiallyConvertRuntimeException(e);
throw potentiallyConvertRuntimeException(e, exceptionTranslator);
}
}
@ -1548,10 +1548,17 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -1548,10 +1548,17 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
throw new InvalidDataAccessApiUsageException(String.format("Resource %s not found!", function));
}
Scanner scanner = null;
try {
return new Scanner(functionResource.getInputStream()).useDelimiter("\\A").next();
scanner = new Scanner(functionResource.getInputStream());
return scanner.useDelimiter("\\A").next();
} catch (IOException e) {
throw new InvalidDataAccessApiUsageException(String.format("Cannot read map-reduce file %s!", function), e);
} finally {
if (scanner != null) {
scanner.close();
}
}
}
@ -1814,7 +1821,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -1814,7 +1821,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
prepareCollection(collection);
return collection;
} catch (RuntimeException e) {
throw potentiallyConvertRuntimeException(e);
throw potentiallyConvertRuntimeException(e, exceptionTranslator);
}
}
@ -1840,7 +1847,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -1840,7 +1847,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
collectionName)));
return result;
} catch (RuntimeException e) {
throw potentiallyConvertRuntimeException(e);
throw potentiallyConvertRuntimeException(e, exceptionTranslator);
}
}
@ -1893,7 +1900,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -1893,7 +1900,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
}
}
} catch (RuntimeException e) {
throw potentiallyConvertRuntimeException(e);
throw potentiallyConvertRuntimeException(e, exceptionTranslator);
}
}
@ -1923,7 +1930,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -1923,7 +1930,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
}
} catch (RuntimeException e) {
throw potentiallyConvertRuntimeException(e);
throw potentiallyConvertRuntimeException(e, exceptionTranslator);
}
}
@ -2002,18 +2009,6 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -2002,18 +2009,6 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
}
}
/**
* Tries to convert the given {@link RuntimeException} into a {@link DataAccessException} but returns the original
* exception if the conversation failed. Thus allows safe rethrowing of the return value.
*
* @param ex
* @return
*/
private RuntimeException potentiallyConvertRuntimeException(RuntimeException ex) {
RuntimeException resolved = this.exceptionTranslator.translateExceptionIfPossible(ex);
return resolved == null ? ex : resolved;
}
/**
* Inspects the given {@link CommandResult} for erros and potentially throws an
* {@link InvalidDataAccessApiUsageException} for that error.
@ -2052,6 +2047,20 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -2052,6 +2047,20 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
return queryMapper.getMappedSort(query.getSortObject(), mappingContext.getPersistentEntity(type));
}
/**
* Tries to convert the given {@link RuntimeException} into a {@link DataAccessException} but returns the original
* exception if the conversation failed. Thus allows safe re-throwing of the return value.
*
* @param ex the exception to translate
* @param exceptionTranslator the {@link PersistenceExceptionTranslator} to be used for translation
* @return
*/
private static RuntimeException potentiallyConvertRuntimeException(RuntimeException ex,
PersistenceExceptionTranslator exceptionTranslator) {
RuntimeException resolved = exceptionTranslator.translateExceptionIfPossible(ex);
return resolved == null ? ex : resolved;
}
// Callback implementations
/**
@ -2298,7 +2307,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -2298,7 +2307,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
}
} catch (RuntimeException e) {
throw potentiallyConvertRuntimeException(e);
throw potentiallyConvertRuntimeException(e, exceptionTranslator);
}
return cursorToUse;
@ -2345,20 +2354,20 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -2345,20 +2354,20 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
* @since 1.7
* @author Thomas Darimont
*/
static class CloseableIterableCusorAdapter<T> implements CloseableIterator<T> {
static class CloseableIterableCursorAdapter<T> implements CloseableIterator<T> {
private volatile Cursor cursor;
private PersistenceExceptionTranslator exceptionTranslator;
private DbObjectCallback<T> objectReadCallback;
/**
* Creates a new {@link CloseableIterableCusorAdapter} backed by the given {@link Cursor}.
* Creates a new {@link CloseableIterableCursorAdapter} backed by the given {@link Cursor}.
*
* @param cursor
* @param exceptionTranslator
* @param objectReadCallback
*/
public CloseableIterableCusorAdapter(Cursor cursor, PersistenceExceptionTranslator exceptionTranslator,
public CloseableIterableCursorAdapter(Cursor cursor, PersistenceExceptionTranslator exceptionTranslator,
DbObjectCallback<T> objectReadCallback) {
this.cursor = cursor;
@ -2376,7 +2385,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -2376,7 +2385,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
try {
return cursor.hasNext();
} catch (RuntimeException ex) {
throw exceptionTranslator.translateExceptionIfPossible(ex);
throw potentiallyConvertRuntimeException(ex, exceptionTranslator);
}
}
@ -2392,7 +2401,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -2392,7 +2401,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
T converted = objectReadCallback.doWith(item);
return converted;
} catch (RuntimeException ex) {
throw exceptionTranslator.translateExceptionIfPossible(ex);
throw potentiallyConvertRuntimeException(ex, exceptionTranslator);
}
}
@ -2403,7 +2412,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { @@ -2403,7 +2412,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
try {
c.close();
} catch (RuntimeException ex) {
throw exceptionTranslator.translateExceptionIfPossible(ex);
throw potentiallyConvertRuntimeException(ex, exceptionTranslator);
} finally {
cursor = null;
exceptionTranslator = null;

83
spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/CloseableIterableCursorAdapterUnitTests.java

@ -0,0 +1,83 @@ @@ -0,0 +1,83 @@
/*
* Copyright 2015 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.mongodb.core;
import static org.mockito.Mockito.*;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import org.springframework.dao.support.PersistenceExceptionTranslator;
import org.springframework.data.mongodb.core.MongoTemplate.CloseableIterableCursorAdapter;
import org.springframework.data.mongodb.core.MongoTemplate.DbObjectCallback;
import org.springframework.data.util.CloseableIterator;
import com.mongodb.Cursor;
/**
* Unit tests for {@link CloseableIterableCursorAdapter}.
*
* @author Oliver Gierke
* @see DATAMONGO-1276
*/
@RunWith(MockitoJUnitRunner.class)
public class CloseableIterableCursorAdapterUnitTests {
@Mock PersistenceExceptionTranslator exceptionTranslator;
@Mock DbObjectCallback<Object> callback;
Cursor cursor;
CloseableIterator<Object> adapter;
@Before
public void setUp() {
this.cursor = doThrow(IllegalArgumentException.class).when(mock(Cursor.class));
this.adapter = new CloseableIterableCursorAdapter<Object>(cursor, exceptionTranslator, callback);
}
/**
* @see DATAMONGO-1276
*/
@Test(expected = IllegalArgumentException.class)
public void propagatesOriginalExceptionFromAdapterDotNext() {
cursor.next();
adapter.next();
}
/**
* @see DATAMONGO-1276
*/
@Test(expected = IllegalArgumentException.class)
public void propagatesOriginalExceptionFromAdapterDotHasNext() {
cursor.hasNext();
adapter.hasNext();
}
/**
* @see DATAMONGO-1276
*/
@Test(expected = IllegalArgumentException.class)
public void propagatesOriginalExceptionFromAdapterDotClose() {
cursor.close();
adapter.close();
}
}
Loading…
Cancel
Save