From 32a46d1289a34a7b06cabd87e7d14806937088a2 Mon Sep 17 00:00:00 2001 From: Ilya Chekashkin Date: Mon, 10 Feb 2020 15:53:30 +0400 Subject: [PATCH] Fix retry mechanism for the DocFormatter and PdfConverter (#126) * Fix retry mechanism for the DocFormatter and DocumentConverter --- core/build.gradle | 1 + .../yarg/formatters/impl/DocFormatter.java | 45 ++++++++++------- .../impl/doc/connector/OfficeIntegration.java | 17 +++++-- .../doc/connector/OfficeIntegrationAPI.java | 7 +++ .../impl/xls/DocumentConverterImpl.java | 49 ++++++++++++------ .../core/test/smoketest/DocSpecificTest.java | 50 ++++++++++++++++++- 6 files changed, 130 insertions(+), 39 deletions(-) diff --git a/core/build.gradle b/core/build.gradle index b89a688d..1ea777bb 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -273,6 +273,7 @@ configure(core) { testCompile(group: 'org.jmockit', name: 'jmockit', version: '1.15') testCompile(group: 'junit', name: 'junit', version: '4.5') + testCompile group: 'org.mockito', name: 'mockito-core', version: '3.2.4' testCompile(group: 'com.h2database', name: 'h2', version: '1.4.196') testCompile(group: 'ch.qos.logback', name: 'logback-classic', version: '1.2.3') testCompile(group: 'commons-dbcp', name: 'commons-dbcp', version: '1.4') diff --git a/core/modules/core/src/com/haulmont/yarg/formatters/impl/DocFormatter.java b/core/modules/core/src/com/haulmont/yarg/formatters/impl/DocFormatter.java index 26ab829e..77937f13 100644 --- a/core/modules/core/src/com/haulmont/yarg/formatters/impl/DocFormatter.java +++ b/core/modules/core/src/com/haulmont/yarg/formatters/impl/DocFormatter.java @@ -86,29 +86,40 @@ public DocFormatter(FormatterFactoryInput formatterFactoryInput, OfficeIntegrati public void renderDocument() { try { doCreateDocument(outputStream); - } catch (ReportingInterruptedException ie) { + } catch (ReportingInterruptedException ie) { throw ie; - } catch (Exception e) {//just try again if any exceptions occurred - log.warn(String.format("An error occurred while generating doc report [%s]. System will retry to generate report again.", reportTemplate.getDocumentName()), e); - - for (int i = 0; i < officeIntegration.getCountOfRetry(); i++) { - try { - checkThreadInterrupted(); - doCreateDocument(outputStream); - return; - } catch (NoFreePortsException e1) { - if (e instanceof NoFreePortsException) { - throw (NoFreePortsException) e; - } - } - } - - throw wrapWithReportingException("An error occurred while generating doc report.", e); + } catch (Exception e) { + doCreateDocumentWithRetries(outputStream, e, 0); } finally { IOUtils.closeQuietly(outputStream); } } + protected void doCreateDocumentWithRetries(final OutputStream outputStream, Exception lastException, int currentAttempt) { + if (officeIntegration.getCountOfRetry() != 0 && currentAttempt < officeIntegration.getCountOfRetry()) { + log.warn("An error occurred while generating doc report {}. System will retry to generate doc report again (current attempt: {}).", + reportTemplate.getDocumentName(), currentAttempt + 1); + log.debug("Last error:", lastException); + try { + Thread.sleep(officeIntegration.getRetryIntervalMs()); + + checkThreadInterrupted(); + doCreateDocument(outputStream); + } catch (ReportingInterruptedException ie) { + throw ie; + } catch (InterruptedException e) { + throw new ReportingInterruptedException("Doc report task interrupted"); + } catch (Exception e) { + doCreateDocumentWithRetries(outputStream, e, ++currentAttempt); + } + } else { + if (lastException instanceof NoFreePortsException) { + throw (NoFreePortsException) lastException; + } + throw wrapWithReportingException("An error occurred while generating doc report. All attempts failed", lastException); + } + } + protected void doCreateDocument(final OutputStream outputStream) throws NoFreePortsException { OfficeTask officeTask = ooResourceProvider -> { try { diff --git a/core/modules/core/src/com/haulmont/yarg/formatters/impl/doc/connector/OfficeIntegration.java b/core/modules/core/src/com/haulmont/yarg/formatters/impl/doc/connector/OfficeIntegration.java index a2726c3d..d1696a15 100644 --- a/core/modules/core/src/com/haulmont/yarg/formatters/impl/doc/connector/OfficeIntegration.java +++ b/core/modules/core/src/com/haulmont/yarg/formatters/impl/doc/connector/OfficeIntegration.java @@ -28,14 +28,15 @@ public class OfficeIntegration implements OfficeIntegrationAPI { protected volatile boolean platformDependProcessManagement = true; protected final ExecutorService executor; - protected final BlockingQueue connectionsQueue = new LinkedBlockingDeque(); - protected final Set connections = new CopyOnWriteArraySet(); + protected final BlockingQueue connectionsQueue = new LinkedBlockingDeque<>(); + protected final Set connections = new CopyOnWriteArraySet<>(); protected String openOfficePath; protected String temporaryDirPath; protected Integer[] openOfficePorts; - protected Integer timeoutInSeconds = 60; - protected int countOfRetry = 2; + protected Integer timeoutInSeconds = DEFAULT_TIMEOUT; + protected int countOfRetry = DEFAULT_RETRY_COUNT; + protected int retryIntervalMs = DEFAULT_RETRY_INTERVAL; protected Boolean displayDeviceAvailable = false; public OfficeIntegration(String openOfficePath, Integer... ports) { @@ -61,6 +62,14 @@ public void setCountOfRetry(int countOfRetry) { this.countOfRetry = countOfRetry; } + public int getRetryIntervalMs() { + return retryIntervalMs; + } + + public void setRetryIntervalMs(int retryIntervalMs) { + this.retryIntervalMs = retryIntervalMs; + } + public String getTemporaryDirPath() { return temporaryDirPath; } diff --git a/core/modules/core/src/com/haulmont/yarg/formatters/impl/doc/connector/OfficeIntegrationAPI.java b/core/modules/core/src/com/haulmont/yarg/formatters/impl/doc/connector/OfficeIntegrationAPI.java index d36aa95e..2e9f2965 100644 --- a/core/modules/core/src/com/haulmont/yarg/formatters/impl/doc/connector/OfficeIntegrationAPI.java +++ b/core/modules/core/src/com/haulmont/yarg/formatters/impl/doc/connector/OfficeIntegrationAPI.java @@ -17,12 +17,19 @@ package com.haulmont.yarg.formatters.impl.doc.connector; public interface OfficeIntegrationAPI { + + int DEFAULT_RETRY_COUNT = 2; + int DEFAULT_RETRY_INTERVAL = 1000; + int DEFAULT_TIMEOUT = 60; + String getTemporaryDirPath(); Integer getTimeoutInSeconds(); int getCountOfRetry(); + int getRetryIntervalMs(); + Boolean isDisplayDeviceAvailable(); void runTaskWithTimeout(OfficeTask officeTask, int timeoutInSeconds) throws NoFreePortsException; diff --git a/core/modules/core/src/com/haulmont/yarg/formatters/impl/xls/DocumentConverterImpl.java b/core/modules/core/src/com/haulmont/yarg/formatters/impl/xls/DocumentConverterImpl.java index 7ba8ec6e..e9757a8e 100644 --- a/core/modules/core/src/com/haulmont/yarg/formatters/impl/xls/DocumentConverterImpl.java +++ b/core/modules/core/src/com/haulmont/yarg/formatters/impl/xls/DocumentConverterImpl.java @@ -45,37 +45,54 @@ public DocumentConverterImpl(OfficeIntegrationAPI officeIntegration) { public void convertToPdf(FileType fileType, final byte[] documentBytes, final OutputStream outputStream) { String convertPattern = FileType.SPREADSHEET == fileType ? XLS_TO_PDF_OUTPUT_FILE : ODT_TO_PDF_OUTPUT_FILE; - convertWithRetry(convertPattern, documentBytes, outputStream); + convert(convertPattern, documentBytes, outputStream); } @Override public void convertToHtml(FileType fileType, byte[] documentBytes, OutputStream outputStream) { String convertPattern = FileType.SPREADSHEET == fileType ? XLS_TO_HTML_OUTPUT_FILE : ODT_TO_HTML_OUTPUT_FILE; - convertWithRetry(convertPattern, documentBytes, outputStream); + convert(convertPattern, documentBytes, outputStream); } - protected void convertWithRetry(String convertPattern, final byte[] documentBytes, final OutputStream outputStream) { + protected void convert(String convertPattern, final byte[] documentBytes, final OutputStream outputStream) { try { convertOnes(convertPattern, documentBytes, outputStream); } catch (ReportingInterruptedException e) { throw e; } catch (Exception e) { - log.warn("An error occurred while converting. System will retry to generate report again.", e); - for (int i = 0; i < officeIntegration.getCountOfRetry(); i++) { - try { - if (Thread.interrupted()) { - throw new ReportingInterruptedException("Document conversation task interrupted"); - } - convertOnes(convertPattern, documentBytes, outputStream); - return; - } catch (NoFreePortsException e1) { - if (e instanceof NoFreePortsException) { - throw (NoFreePortsException) e; - } + convertWithRetries(convertPattern, documentBytes, outputStream, e, 0); + } + } + + protected void convertWithRetries(final String convertPattern, + final byte[] documentBytes, + final OutputStream outputStream, + Exception lastException, + int retriesCount) { + if (officeIntegration.getCountOfRetry() != 0 && retriesCount < officeIntegration.getCountOfRetry()) { + log.warn("An error occurred while converting to {}. System will retry to convert again (Current attempt: {}).", + convertPattern, retriesCount + 1); + log.debug("Last error:", lastException); + try { + Thread.sleep(officeIntegration.getRetryIntervalMs()); + + if (Thread.interrupted()) { + throw new ReportingInterruptedException("Document conversation task interrupted"); } + convertOnes(convertPattern, documentBytes, outputStream); + } catch (ReportingInterruptedException ie) { + throw ie; + } catch (InterruptedException e) { + throw new ReportingInterruptedException("Document conversation task interrupted"); + } catch (Exception e) { + convertWithRetries(convertPattern, documentBytes, outputStream, e, ++retriesCount); + } + } else { + if (lastException instanceof NoFreePortsException) { + throw (NoFreePortsException) lastException; } - throw new ReportingException("An error occurred while converting.", e); + throw new ReportingException(String.format("Unable to convert to %s. All attempts failed", convertPattern), lastException); } } diff --git a/core/modules/core/test/smoketest/DocSpecificTest.java b/core/modules/core/test/smoketest/DocSpecificTest.java index 096f15f4..3511e748 100644 --- a/core/modules/core/test/smoketest/DocSpecificTest.java +++ b/core/modules/core/test/smoketest/DocSpecificTest.java @@ -18,10 +18,13 @@ import com.haulmont.yarg.formatters.ReportFormatter; import com.haulmont.yarg.formatters.factory.DefaultFormatterFactory; import com.haulmont.yarg.formatters.factory.FormatterFactoryInput; +import com.haulmont.yarg.formatters.impl.DocFormatter; import com.haulmont.yarg.formatters.impl.doc.connector.OfficeIntegration; +import com.haulmont.yarg.formatters.impl.doc.connector.OfficeIntegrationAPI; +import com.haulmont.yarg.formatters.impl.doc.connector.OfficeTask; +import com.haulmont.yarg.formatters.impl.xls.DocumentConverterImpl; import com.haulmont.yarg.structure.BandData; import com.haulmont.yarg.structure.BandOrientation; -import com.haulmont.yarg.structure.ReportFieldFormat; import com.haulmont.yarg.structure.ReportOutputType; import com.haulmont.yarg.structure.impl.ReportFieldFormatImpl; import com.haulmont.yarg.structure.impl.ReportTemplateImpl; @@ -35,7 +38,15 @@ import java.util.HashMap; import java.util.concurrent.CountDownLatch; -public class DocSpecificTest extends AbstractFormatSpecificTest{ +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.*; + +/** + * @author degtyarjov + * @version $Id$ + */ +public class DocSpecificTest extends AbstractFormatSpecificTest { @Test public void testTableOdt() throws Exception { BandData root = new BandData("Root"); @@ -192,4 +203,39 @@ public void run() { countDownLatch.await(); System.out.println(); } + + @Test + public void testRetryCount() throws Exception { + BandData root = createRootBand(); + FormatterFactoryInput formatterFactoryInput = new FormatterFactoryInput("doc", root, + new ReportTemplateImpl("", "./modules/core/test/smoketest/test.doc", + "./modules/core/test/smoketest/test.doc", ReportOutputType.doc), null); + OfficeIntegrationAPI officeIntegrationApiMock = createOfficeIntegrationApiMock(5); + DocFormatter docFormatter = new DocFormatter(formatterFactoryInput, officeIntegrationApiMock); + + // Check that in case of exception 'runTaskWithTimeout' will execute exactly retriesCount + 1 times + try { + docFormatter.renderDocument(); + } catch (Exception e) { + verify(officeIntegrationApiMock, times(6)) + .runTaskWithTimeout((OfficeTask) any(), anyInt()); + } + + officeIntegrationApiMock = createOfficeIntegrationApiMock(4); + DocumentConverterImpl pdfConverter = new DocumentConverterImpl(officeIntegrationApiMock); + try { + pdfConverter.convertToPdf(DocumentConverterImpl.FileType.DOCUMENT, null, null); + } catch (Exception e) { + verify(officeIntegrationApiMock, times(5)) + .runTaskWithTimeout((OfficeTask) any(), anyInt()); + } + } + + protected OfficeIntegrationAPI createOfficeIntegrationApiMock(int retryCount) { + OfficeIntegrationAPI mOfficeIntegrationApi = mock(OfficeIntegrationAPI.class); + doThrow(RuntimeException.class).when(mOfficeIntegrationApi).runTaskWithTimeout((OfficeTask) any(), anyInt()); + when(mOfficeIntegrationApi.getCountOfRetry()).thenReturn(retryCount); + when(mOfficeIntegrationApi.getRetryIntervalMs()).thenReturn(300); + return mOfficeIntegrationApi; + } } \ No newline at end of file