Skip to content
Snippets Groups Projects
Commit 0c926627 authored by Krzysztof Witukiewicz's avatar Krzysztof Witukiewicz
Browse files

OZG-7262 OZG-7627 Don't call sendChunks() after exception

parent 844a1a87
No related branches found
No related tags found
1 merge request!6OZG-7262 OZG-7566 Notify callObserver about error
Show whitespace changes
Inline Side-by-side
ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloader.java
+ 15
− 16
View file @ 0c926627
......@@ -57,7 +57,7 @@ public class GrpcBinaryFileServerDownloader<T> {
private final AtomicBoolean started = new AtomicBoolean(false);
private final AtomicBoolean downloadFinished = new AtomicBoolean(false);
private final AtomicBoolean requestFinished = new AtomicBoolean(false);
private final AtomicReference<TechnicalException> error = new AtomicReference<>();
private final AtomicReference<TechnicalException> downloadError = new AtomicReference<>();
private PipedInputStream inputStream;
private PipedOutputStream outputStream;
......@@ -107,11 +107,12 @@ public class GrpcBinaryFileServerDownloader<T> {
void startDownload() {
try {
doDownload();
sendChunks();
} catch (Exception e) {
error.set(new TechnicalException("Error while downloading file contents", e));
LOG.error("Error while downloading file contents", e);
downloadError.set(new TechnicalException("Error while downloading file contents", e));
} finally {
closeOutputStream();
sendChunks();
}
}
......@@ -136,10 +137,6 @@ public class GrpcBinaryFileServerDownloader<T> {
}
int bytesRead;
while (isReady()) {
if (Objects.nonNull(error.get())) {
completeRequestWithError(error.get());
break;
}
if ((bytesRead = inputStream.read(buffer)) == -1) {
tryCompleteRequest();
break;
......@@ -154,26 +151,28 @@ public class GrpcBinaryFileServerDownloader<T> {
}
void tryCompleteRequest() {
if (shouldCompleteRequest()) {
completeRequest();
if (Objects.nonNull(downloadError.get())) {
throw downloadError.get();
} else if (downloadFinished.get()) {
completeRequestNormally();
}
}
boolean shouldCompleteRequest() {
return downloadFinished.get() && requestFinished.compareAndSet(false, true);
void completeRequestWithError(TechnicalException e) {
LOG.debug("Complete download request with error");
finishRequest();
throw e;
}
void completeRequest() {
void completeRequestNormally() {
LOG.debug("Complete download request");
closeInputStream();
finishRequest();
callObserver.onCompleted();
}
void completeRequestWithError(TechnicalException e) {
LOG.debug("Complete download request with error", e);
private void finishRequest() {
requestFinished.set(true);
closeInputStream();
callObserver.onError(e);
}
void closeOutputStream() {
......
ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloaderTest.java
+ 88
− 151
View file @ 0c926627
......@@ -229,7 +229,7 @@ class GrpcBinaryFileServerDownloaderTest {
@Test
void shouldErrorBeInitiallyNull() {
assertThat(getError()).isNull();
assertThat(getDownloadError()).isNull();
}
@Nested
......@@ -256,7 +256,6 @@ class GrpcBinaryFileServerDownloaderTest {
}
@Test
@DisplayName("should send chunks here to not wait for callObserver to change its ready status")
void shouldSendChunks() {
downloader.startDownload();
......@@ -278,7 +277,7 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldSetError() {
downloader.startDownload();
assertThat(getError()).isInstanceOf(TechnicalException.class).hasCause(exception);
assertThat(getDownloadError()).isInstanceOf(TechnicalException.class).hasCause(exception);
}
@SneakyThrows
......@@ -288,14 +287,6 @@ class GrpcBinaryFileServerDownloaderTest {
verify(outputStream).close();
}
@Test
@DisplayName("should send chunks here to communicate error to callObserver")
void shouldSendChunks() {
downloader.startDownload();
verify(downloader).sendChunks();
}
}
}
......@@ -362,6 +353,7 @@ class GrpcBinaryFileServerDownloaderTest {
@BeforeEach
void init() {
doThrow(exception).when(downloader).doSendChunks();
doNothing().when(downloader).completeRequestWithError(any());
}
@Test
......@@ -416,40 +408,6 @@ class GrpcBinaryFileServerDownloaderTest {
@Nested
class OnReady {
@BeforeEach
void init() {
when(callObserver.isReady()).thenReturn(true);
}
@Nested
class OnHasError {
private final TechnicalException exception = new TechnicalException("error");
@BeforeEach
void init() {
setErrorField(exception);
doNothing().when(downloader).completeRequestWithError(any());
}
@Test
void shouldOnlyCallIsReadyOnObserver() {
doSendChunks();
verify(callObserver).isReady();
verifyNoMoreInteractions(callObserver);
}
@Test
void shouldCompleteRequestWithError() {
doSendChunks();
verify(downloader).completeRequestWithError(exception);
}
}
@Nested
class OnHasNoError {
@Mock
private PipedInputStream inputStream;
@Captor
......@@ -499,7 +457,6 @@ class GrpcBinaryFileServerDownloaderTest {
}
}
}
}
@SneakyThrows
private void doSendChunks() {
......@@ -510,69 +467,48 @@ class GrpcBinaryFileServerDownloaderTest {
@Nested
class TestTryCompleteRequest {
@Test
void shouldCallShouldCompleteRequest() {
downloader.tryCompleteRequest();
verify(downloader).shouldCompleteRequest();
}
@Test
void shouldCallCompleteRequest() {
doReturn(true).when(downloader).shouldCompleteRequest();
@Nested
class OnError {
downloader.tryCompleteRequest();
private final TechnicalException exception = new TechnicalException("error");
verify(downloader).completeRequest();
@BeforeEach
void init() {
setDownloadErrorField(exception);
}
@Test
void shouldNotCallCompleteRequest() {
doReturn(false).when(downloader).shouldCompleteRequest();
downloader.tryCompleteRequest();
verify(downloader, never()).completeRequest();
void shouldThrowException() {
assertThatThrownBy(downloader::tryCompleteRequest).isSameAs(exception);
}
}
@Nested
class TestShouldCompleteRequest {
@Nested
class TestWhenDownloadFinished {
class OnDownloadFinished {
@BeforeEach
void init() {
setDownloadFinishedField(true);
doNothing().when(downloader).completeRequestNormally();
}
@Test
void shouldReturnTrue() {
var result = downloader.shouldCompleteRequest();
assertThat(result).isTrue();
}
@Test
void shouldReturnFalseIfRequestFinished() {
setRequestFinishedField(true);
var result = downloader.shouldCompleteRequest();
void shouldNotCompleteRequestWithError() {
downloader.tryCompleteRequest();
assertThat(result).isFalse();
verify(downloader, never()).completeRequestWithError(any());
}
@Test
void shouldUpdateRequestFinished() {
downloader.shouldCompleteRequest();
void shouldCompleteRequestNormally() {
downloader.tryCompleteRequest();
assertThat(getRequestFinished()).isTrue();
verify(downloader).completeRequestNormally();
}
}
@Nested
class TestWhenDownloadRunning {
class OnDownloadNotFinished {
@BeforeEach
void init() {
......@@ -580,45 +516,48 @@ class GrpcBinaryFileServerDownloaderTest {
}
@Test
void shouldReturnFalse() {
var result = downloader.shouldCompleteRequest();
void shouldNotCompleteRequestNormally() {
downloader.tryCompleteRequest();
assertThat(result).isFalse();
verify(downloader, never()).completeRequestNormally();
}
@Test
void shouldNotUpdateRequestFinished() {
downloader.shouldCompleteRequest();
void shouldNotCompleteRequestWithError() {
downloader.tryCompleteRequest();
assertThat(getRequestFinished()).isFalse();
verify(downloader, never()).completeRequestWithError(any());
}
}
}
@Nested
class TestCompleteRequest {
@Mock
private PipedInputStream inputStream;
class TestCompleteRequestNormally {
@BeforeEach
void mock() {
setRequestFinishedField(false);
setDownloadFinishedField(true);
setInputStreamField(inputStream);
void init() {
doNothing().when(downloader).closeInputStream();
}
@SneakyThrows
@Test
void shouldCallCloseInputStream() {
downloader.completeRequest();
void shouldSetRequestFinished() {
assertThat(getRequestFinished()).isFalse();
downloader.completeRequestNormally();
assertThat(getRequestFinished()).isTrue();
}
@Test
void shouldCloseInputStream() {
downloader.completeRequestNormally();
verify(downloader).closeInputStream();
}
@Test
void shouldCallOnCompleted() {
downloader.completeRequest();
void shouldNotifyObserver() {
downloader.completeRequestNormally();
verify(callObserver).onCompleted();
}
......@@ -638,23 +577,21 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldSetRequestFinished() {
assertThat(getRequestFinished()).isFalse();
downloader.completeRequestWithError(error);
catchException(() -> downloader.completeRequestWithError(error));
assertThat(getRequestFinished()).isTrue();
}
@Test
void shouldCloseInputStream() {
downloader.completeRequestWithError(error);
catchException(() -> downloader.completeRequestWithError(error));
verify(downloader).closeInputStream();
}
@Test
void shouldNotifyCallObserver() {
downloader.completeRequestWithError(error);
verify(callObserver).onError(error);
void shouldThrowException() {
assertThatThrownBy(() -> downloader.completeRequestWithError(error)).isSameAs(error);
}
}
......@@ -674,12 +611,12 @@ class GrpcBinaryFileServerDownloaderTest {
return ReflectionTestUtils.getField(downloader, "requestFinished", AtomicBoolean.class).get();
}
private void setErrorField(TechnicalException error) {
ReflectionTestUtils.setField(downloader, "error", new AtomicReference<>(error));
private void setDownloadErrorField(TechnicalException error) {
ReflectionTestUtils.setField(downloader, "downloadError", new AtomicReference<>(error));
}
private TechnicalException getError() {
return (TechnicalException) ReflectionTestUtils.getField(downloader, "error", AtomicReference.class).get();
private TechnicalException getDownloadError() {
return (TechnicalException) ReflectionTestUtils.getField(downloader, "downloadError", AtomicReference.class).get();
}
private void setDownloadFinishedField(boolean downloadFinished) {
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment

Impressum - Datenschutz - Barrierefreiheit