Merge branch 'ATS-176' into 'master'

ATS-176 : Log improvements with Slf4j

See merge request Repository/alfresco-docker-transformers!29
This commit is contained in:
Cezar Leahu 2018-11-13 15:22:38 +00:00
commit edaed0a4b2
15 changed files with 160 additions and 164 deletions

View File

@ -28,8 +28,8 @@ import javax.servlet.http.HttpServletRequest;
import org.alfresco.transformer.executors.PdfRendererCommandExecutor; import org.alfresco.transformer.executors.PdfRendererCommandExecutor;
import org.alfresco.transformer.logging.LogEntry; import org.alfresco.transformer.logging.LogEntry;
import org.alfresco.transformer.probes.ProbeTestTransform; import org.alfresco.transformer.probes.ProbeTestTransform;
import org.apache.commons.logging.Log; import org.slf4j.Logger;
import org.apache.commons.logging.LogFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.io.Resource; import org.springframework.core.io.Resource;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
@ -62,7 +62,7 @@ import org.springframework.web.multipart.MultipartFile;
@Controller @Controller
public class AlfrescoPdfRendererController extends AbstractTransformerController public class AlfrescoPdfRendererController extends AbstractTransformerController
{ {
private static final Log logger = LogFactory.getLog(AlfrescoPdfRendererController.class); private static final Logger logger = LoggerFactory.getLogger(AlfrescoPdfRendererController.class);
@Autowired @Autowired
private PdfRendererCommandExecutor commandExecutor; private PdfRendererCommandExecutor commandExecutor;
@ -92,8 +92,8 @@ public class AlfrescoPdfRendererController extends AbstractTransformerController
public ProbeTestTransform getProbeTestTransform() public ProbeTestTransform getProbeTestTransform()
{ {
// See the Javadoc on this method and Probes.md for the choice of these values. // See the Javadoc on this method and Probes.md for the choice of these values.
return new ProbeTestTransform(this, logger, "quick.pdf", "quick.png", return new ProbeTestTransform(this, "quick.pdf", "quick.png",
7455, 1024, 150, 10240, 60*20+1, 60*15-15) 7455, 1024, 150, 10240, 60 * 20 + 1, 60 * 15 - 15)
{ {
@Override @Override
protected void executeTransformCommand(File sourceFile, File targetFile) protected void executeTransformCommand(File sourceFile, File targetFile)
@ -107,6 +107,9 @@ public class AlfrescoPdfRendererController extends AbstractTransformerController
public void processTransform(File sourceFile, File targetFile, public void processTransform(File sourceFile, File targetFile,
Map<String, String> transformOptions, Long timeout) Map<String, String> transformOptions, Long timeout)
{ {
logger.debug("Processing request with: sourceFile '{}', targetFile '{}', transformOptions" +
" '{}', timeout {} ms", sourceFile, targetFile, transformOptions, timeout);
String page = transformOptions.get("page"); String page = transformOptions.get("page");
Integer pageOption = page == null ? null : Integer.parseInt(page); Integer pageOption = page == null ? null : Integer.parseInt(page);
@ -162,9 +165,8 @@ public class AlfrescoPdfRendererController extends AbstractTransformerController
return body; return body;
} }
private static String buildTransformOptions(Integer page, Integer width, Integer height, Boolean
private static String buildTransformOptions(Integer page,Integer width,Integer height,Boolean allowEnlargement, Boolean maintainAspectRatio)
allowEnlargement,Boolean maintainAspectRatio)
{ {
StringJoiner args = new StringJoiner(" "); StringJoiner args = new StringJoiner(" ");
if (width != null && width >= 0) if (width != null && width >= 0)

View File

@ -25,11 +25,11 @@ import java.util.Map;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.alfresco.transformer.executors.ImageMagickCommandExecutor; import org.alfresco.transformer.executors.ImageMagickCommandExecutor;
import org.alfresco.transformer.logging.LogEntry; import org.alfresco.transformer.logging.LogEntry;
import org.alfresco.transformer.probes.ProbeTestTransform; import org.alfresco.transformer.probes.ProbeTestTransform;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.io.Resource; import org.springframework.core.io.Resource;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
@ -64,7 +64,7 @@ import org.springframework.web.multipart.MultipartFile;
@Controller @Controller
public class ImageMagickController extends AbstractTransformerController public class ImageMagickController extends AbstractTransformerController
{ {
private static final Log logger = LogFactory.getLog(ImageMagickController.class); private static final Logger logger = LoggerFactory.getLogger(ImageMagickController.class);
@Autowired @Autowired
private ImageMagickCommandExecutor commandExecutor; private ImageMagickCommandExecutor commandExecutor;
@ -94,7 +94,7 @@ public class ImageMagickController extends AbstractTransformerController
public ProbeTestTransform getProbeTestTransform() public ProbeTestTransform getProbeTestTransform()
{ {
// See the Javadoc on this method and Probes.md for the choice of these values. // See the Javadoc on this method and Probes.md for the choice of these values.
return new ProbeTestTransform(this, logger, "quick.jpg", "quick.png", return new ProbeTestTransform(this, "quick.jpg", "quick.png",
35593, 1024, 150, 1024, 60*15+1,60*15) 35593, 1024, 150, 1024, 60*15+1,60*15)
{ {
@Override @Override
@ -189,6 +189,9 @@ public class ImageMagickController extends AbstractTransformerController
public void processTransform(final File sourceFile, final File targetFile, public void processTransform(final File sourceFile, final File targetFile,
final Map<String, String> transformOptions, final Long timeout) final Map<String, String> transformOptions, final Long timeout)
{ {
logger.debug("Processing request with: sourceFile '{}', targetFile '{}', transformOptions" +
" '{}', timeout {} ms", sourceFile, targetFile, transformOptions, timeout);
final String options = OptionsBuilder final String options = OptionsBuilder
.builder() .builder()
.withStartPage(transformOptions.get("startPage")) .withStartPage(transformOptions.get("startPage"))

View File

@ -27,8 +27,8 @@ import javax.servlet.http.HttpServletRequest;
import org.alfresco.transformer.executors.LibreOfficeJavaExecutor; import org.alfresco.transformer.executors.LibreOfficeJavaExecutor;
import org.alfresco.transformer.logging.LogEntry; import org.alfresco.transformer.logging.LogEntry;
import org.alfresco.transformer.probes.ProbeTestTransform; import org.alfresco.transformer.probes.ProbeTestTransform;
import org.apache.commons.logging.Log; import org.slf4j.Logger;
import org.apache.commons.logging.LogFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.io.Resource; import org.springframework.core.io.Resource;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
@ -62,7 +62,7 @@ import org.springframework.web.multipart.MultipartFile;
@Controller @Controller
public class LibreOfficeController extends AbstractTransformerController public class LibreOfficeController extends AbstractTransformerController
{ {
private static final Log logger = LogFactory.getLog(LibreOfficeController.class); private static final Logger logger = LoggerFactory.getLogger(LibreOfficeController.class);
@Autowired @Autowired
private LibreOfficeJavaExecutor javaExecutor; private LibreOfficeJavaExecutor javaExecutor;
@ -92,7 +92,7 @@ public class LibreOfficeController extends AbstractTransformerController
public ProbeTestTransform getProbeTestTransform() public ProbeTestTransform getProbeTestTransform()
{ {
// See the Javadoc on this method and Probes.md for the choice of these values. // See the Javadoc on this method and Probes.md for the choice of these values.
return new ProbeTestTransform(this, logger, "quick.doc", "quick.pdf", return new ProbeTestTransform(this, "quick.doc", "quick.pdf",
11817, 1024, 150, 10240, 60*30+1, 60*15+20) 11817, 1024, 150, 10240, 60*30+1, 60*15+20)
{ {
@Override @Override
@ -131,6 +131,9 @@ public class LibreOfficeController extends AbstractTransformerController
public void processTransform(File sourceFile, File targetFile, public void processTransform(File sourceFile, File targetFile,
Map<String, String> transformOptions, Long timeout) Map<String, String> transformOptions, Long timeout)
{ {
logger.debug("Processing request with: sourceFile '{}', targetFile '{}', transformOptions" +
" '{}', timeout {} ms", sourceFile, targetFile, transformOptions, timeout);
javaExecutor.call(sourceFile, targetFile); javaExecutor.call(sourceFile, targetFile);
} }
} }

View File

@ -37,11 +37,11 @@ import javax.annotation.PostConstruct;
import javax.annotation.PreDestroy; import javax.annotation.PreDestroy;
import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.error.AlfrescoRuntimeException;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.artofsolving.jodconverter.office.DefaultOfficeManagerConfiguration; import org.artofsolving.jodconverter.office.DefaultOfficeManagerConfiguration;
import org.artofsolving.jodconverter.office.OfficeException; import org.artofsolving.jodconverter.office.OfficeException;
import org.artofsolving.jodconverter.office.OfficeManager; import org.artofsolving.jodconverter.office.OfficeManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
///////// THIS FILE WAS A COPY OF THE CODE IN alfresco-repository ///////////// ///////// THIS FILE WAS A COPY OF THE CODE IN alfresco-repository /////////////
@ -53,7 +53,7 @@ import org.artofsolving.jodconverter.office.OfficeManager;
*/ */
public class JodConverterSharedInstance implements JodConverter public class JodConverterSharedInstance implements JodConverter
{ {
private static final Log logger = LogFactory.getLog(JodConverterSharedInstance.class); private static final Logger logger = LoggerFactory.getLogger(JodConverterSharedInstance.class);
private OfficeManager officeManager; private OfficeManager officeManager;
private boolean isAvailable = false; private boolean isAvailable = false;
@ -137,10 +137,7 @@ public class JodConverterSharedInstance implements JodConverter
{ {
// Logging this as an error as this property would prevent JodConverter & therefore // Logging this as an error as this property would prevent JodConverter & therefore
// OOo from starting as specified // OOo from starting as specified
if (logger.isErrorEnabled()) logger.error("Unparseable value for property '{}.portNumbers': {}", sys, s);
{
logger.error("Unparseable value for property '" + sys + ".portNumbers': " + s);
}
// We'll not rethrow the exception, instead allowing the problem to be picked up // We'll not rethrow the exception, instead allowing the problem to be picked up
// when the OOoJodConverter subsystem is started. // when the OOoJodConverter subsystem is started.
} }
@ -268,10 +265,7 @@ public class JodConverterSharedInstance implements JodConverter
} }
catch (NumberFormatException nfe) catch (NumberFormatException nfe)
{ {
if (logger.isDebugEnabled()) logger.debug("Cannot parse numerical value from {}", string);
{
logger.debug("Cannot parse numerical value from " + string);
}
} }
return null; return null;
} }
@ -300,20 +294,20 @@ public class JodConverterSharedInstance implements JodConverter
if (logger.isDebugEnabled()) if (logger.isDebugEnabled())
{ {
logger.debug("JodConverter settings (null settings will be replaced by jodconverter defaults):"); logger.debug("JodConverter settings (null settings will be replaced by jodconverter defaults):");
logger.debug(" officeHome = " + officeHome); logger.debug(" officeHome = {}", officeHome);
logger.debug(" enabled = " + isEnabled()); logger.debug(" enabled = {}", isEnabled());
logger.debug(" portNumbers = " + getString(portNumbers)); logger.debug(" portNumbers = {}", getString(portNumbers));
logger.debug(" ooo.exe = " + deprecatedOooExe); logger.debug(" ooo.exe = {}", deprecatedOooExe);
logger.debug(" ooo.enabled = " + deprecatedOooEnabled); logger.debug(" ooo.enabled = {}", deprecatedOooEnabled);
logger.debug(" ooo.port = " + getString(deprecatedOooPortNumbers)); logger.debug(" ooo.port = {}", getString(deprecatedOooPortNumbers));
logger.debug(" jodConverter.enabled = " + enabled); logger.debug(" jodConverter.enabled = {}", enabled);
logger.debug(" jodconverter.portNumbers = " + getString(this.portNumbers)); logger.debug(" jodconverter.portNumbers = {}", getString(this.portNumbers));
logger.debug(" jodconverter.officeHome = " + this.officeHome); logger.debug(" jodconverter.officeHome = {}", this.officeHome);
logger.debug(" jodconverter.maxTasksPerProcess = " + maxTasksPerProcess); logger.debug(" jodconverter.maxTasksPerProcess = {}", maxTasksPerProcess);
logger.debug(" jodconverter.taskExecutionTimeout = " + taskExecutionTimeout); logger.debug(" jodconverter.taskExecutionTimeout = {}", taskExecutionTimeout);
logger.debug(" jodconverter.taskQueueTimeout = " + taskQueueTimeout); logger.debug(" jodconverter.taskQueueTimeout = {}", taskQueueTimeout);
logger.debug(" jodconverter.connectTimeout = " + connectTimeout); logger.debug(" jodconverter.connectTimeout = {}", connectTimeout);
logger.debug(" jodconverter.url = " + url); logger.debug(" jodconverter.url = {}", url);
} }
// Only start the JodConverter instance(s) if the subsystem is enabled. // Only start the JodConverter instance(s) if the subsystem is enabled.
@ -362,33 +356,25 @@ public class JodConverterSharedInstance implements JodConverter
officeManager = defaultOfficeMgrConfig.buildOfficeManager(); officeManager = defaultOfficeMgrConfig.buildOfficeManager();
officeManager.start(); officeManager.start();
} }
catch (IllegalStateException isx) catch (IllegalStateException e)
{ {
if (logger.isErrorEnabled()) logger.error("Unable to pre-initialise JodConverter library. " +
{ "The following error is shown for informational purposes only.", e);
logger.error("Unable to pre-initialise JodConverter library. "
+ "The following error is shown for informational purposes only.", isx);
}
return; return;
} }
catch (OfficeException ox) catch (OfficeException e)
{ {
if (logger.isErrorEnabled()) logger.error("Unable to start JodConverter library. " +
{ "The following error is shown for informational purposes only.", e);
logger.error("Unable to start JodConverter library. "
+ "The following error is shown for informational purposes only.", ox);
}
// We need to let it continue (comment-out return statement) even if an error occurs. See MNT-13706 and associated issues. // We need to let it continue (comment-out return statement) even if an error occurs. See MNT-13706 and associated issues.
//return; //return;
} }
catch (Exception x) catch (Exception e)
{ {
if (logger.isErrorEnabled()) logger.error(
{ "Unexpected error in configuring or starting the JodConverter library." +
logger.error("Unexpected error in configuring or starting the JodConverter library." "The following error is shown for informational purposes only.", e);
+ "The following error is shown for informational purposes only.", x);
}
return; return;
} }
} }

View File

@ -9,8 +9,8 @@ import java.io.IOException;
import javax.annotation.PostConstruct; import javax.annotation.PostConstruct;
import org.alfresco.transformer.exceptions.TransformException; import org.alfresco.transformer.exceptions.TransformException;
import org.apache.commons.logging.Log; import org.slf4j.Logger;
import org.apache.commons.logging.LogFactory; import org.slf4j.LoggerFactory;
import org.apache.pdfbox.pdmodel.PDDocument; import org.apache.pdfbox.pdmodel.PDDocument;
import org.apache.pdfbox.pdmodel.PDPage; import org.apache.pdfbox.pdmodel.PDPage;
import org.apache.pdfbox.pdmodel.PDPageContentStream; import org.apache.pdfbox.pdmodel.PDPageContentStream;
@ -28,7 +28,7 @@ import com.sun.star.task.ErrorCodeIOException;
@Component @Component
public class LibreOfficeJavaExecutor implements JavaExecutor public class LibreOfficeJavaExecutor implements JavaExecutor
{ {
private static final Log logger = LogFactory.getLog(LibreOfficeJavaExecutor.class); private static final Logger logger = LoggerFactory.getLogger(LibreOfficeJavaExecutor.class);
private static final int JODCONVERTER_TRANSFORMATION_ERROR_CODE = 3088; private static final int JODCONVERTER_TRANSFORMATION_ERROR_CODE = 3088;
private static final String OFFICE_HOME = "/opt/libreoffice5.4"; private static final String OFFICE_HOME = "/opt/libreoffice5.4";

View File

@ -49,17 +49,6 @@
<artifactId>alfresco-data-model</artifactId> <artifactId>alfresco-data-model</artifactId>
</dependency> </dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>1.7.24</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-log4j12</artifactId>
<version>1.7.25</version>
</dependency>
<!-- Tika --> <!-- Tika -->
<dependency> <dependency>
<groupId>org.apache.tika</groupId> <groupId>org.apache.tika</groupId>

View File

@ -37,8 +37,8 @@ import org.alfresco.transformer.exceptions.TransformException;
import org.alfresco.transformer.executors.TikaJavaExecutor; import org.alfresco.transformer.executors.TikaJavaExecutor;
import org.alfresco.transformer.logging.LogEntry; import org.alfresco.transformer.logging.LogEntry;
import org.alfresco.transformer.probes.ProbeTestTransform; import org.alfresco.transformer.probes.ProbeTestTransform;
import org.apache.commons.logging.Log; import org.slf4j.Logger;
import org.apache.commons.logging.LogFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.io.Resource; import org.springframework.core.io.Resource;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
@ -72,7 +72,7 @@ import org.springframework.web.multipart.MultipartFile;
@Controller @Controller
public class TikaController extends AbstractTransformerController public class TikaController extends AbstractTransformerController
{ {
private static final Log logger = LogFactory.getLog(TikaController.class); private static final Logger logger = LoggerFactory.getLogger(TikaController.class);
@Autowired @Autowired
private TikaJavaExecutor javaExecutor; private TikaJavaExecutor javaExecutor;
@ -103,7 +103,7 @@ public class TikaController extends AbstractTransformerController
{ {
// See the Javadoc on this method and Probes.md for the choice of these values. // See the Javadoc on this method and Probes.md for the choice of these values.
// the livenessPercentage is a little large as Tika does tend to suffer from slow transforms that class with a gc. // the livenessPercentage is a little large as Tika does tend to suffer from slow transforms that class with a gc.
return new ProbeTestTransform(this, logger, "quick.pdf", "quick.txt", return new ProbeTestTransform(this, "quick.pdf", "quick.txt",
60, 16, 400, 10240, 60 * 30 + 1, 60 * 15 + 20) 60, 16, 400, 10240, 60 * 30 + 1, 60 * 15 + 20)
{ {
@Override @Override
@ -160,6 +160,8 @@ public class TikaController extends AbstractTransformerController
public void processTransform(File sourceFile, File targetFile, public void processTransform(File sourceFile, File targetFile,
Map<String, String> transformOptions, Long timeout) Map<String, String> transformOptions, Long timeout)
{ {
logger.debug("Processing request with: sourceFile '{}', targetFile '{}', transformOptions" +
" '{}', timeout {} ms", sourceFile, targetFile, transformOptions, timeout);
String transform = transformOptions.get("transform"); String transform = transformOptions.get("transform");
Boolean includeContents = stringToBoolean("includeContents"); Boolean includeContents = stringToBoolean("includeContents");

View File

@ -44,8 +44,8 @@ import org.alfresco.transformer.exceptions.TransformException;
import org.alfresco.transformer.logging.LogEntry; import org.alfresco.transformer.logging.LogEntry;
import org.alfresco.transformer.model.FileRefResponse; import org.alfresco.transformer.model.FileRefResponse;
import org.alfresco.util.TempFileProvider; import org.alfresco.util.TempFileProvider;
import org.apache.commons.logging.Log; import org.slf4j.Logger;
import org.apache.commons.logging.LogFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.io.Resource; import org.springframework.core.io.Resource;
import org.springframework.http.HttpHeaders; import org.springframework.http.HttpHeaders;
@ -91,7 +91,7 @@ import org.springframework.web.client.HttpClientErrorException;
*/ */
public abstract class AbstractTransformerController implements TransformController public abstract class AbstractTransformerController implements TransformController
{ {
private static final Log logger = LogFactory.getLog(AbstractTransformerController.class); private static final Logger logger = LoggerFactory.getLogger(AbstractTransformerController.class);
@Autowired @Autowired
private AlfrescoSharedFileStoreClient alfrescoSharedFileStoreClient; private AlfrescoSharedFileStoreClient alfrescoSharedFileStoreClient;
@ -114,6 +114,8 @@ public abstract class AbstractTransformerController implements TransformControll
public ResponseEntity<TransformReply> transform(@RequestBody TransformRequest request, public ResponseEntity<TransformReply> transform(@RequestBody TransformRequest request,
@RequestParam(value = "timeout", required = false) Long timeout) @RequestParam(value = "timeout", required = false) Long timeout)
{ {
logger.info("Received {}, timeout {} ms", request, timeout);
final TransformReply reply = new TransformReply(); final TransformReply reply = new TransformReply();
reply.setRequestId(request.getRequestId()); reply.setRequestId(request.getRequestId());
reply.setSourceReference(request.getSourceReference()); reply.setSourceReference(request.getSourceReference());
@ -127,8 +129,8 @@ public abstract class AbstractTransformerController implements TransformControll
reply.setErrorDetails(errors.getAllErrors().stream().map(Object::toString) reply.setErrorDetails(errors.getAllErrors().stream().map(Object::toString)
.collect(Collectors.joining(", "))); .collect(Collectors.joining(", ")));
return new ResponseEntity<>(reply, logger.error("Invalid request, sending {}", reply);
HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
// Load the source file // Load the source file
@ -137,51 +139,56 @@ public abstract class AbstractTransformerController implements TransformControll
{ {
sourceFile = loadSourceFile(request.getSourceReference()); sourceFile = loadSourceFile(request.getSourceReference());
} }
catch (TransformException te) catch (TransformException e)
{ {
reply.setStatus(te.getStatusCode()); reply.setStatus(e.getStatusCode());
reply .setErrorDetails("Failed at reading the source file. " + te.getMessage()); reply.setErrorDetails(messageWithCause("Failed at reading the source file", e));
logger.error("Failed to load source file (TransformException), sending " + reply, e);
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
catch (HttpClientErrorException hcee) catch (HttpClientErrorException e)
{ {
reply.setStatus(hcee.getStatusCode().value()); reply.setStatus(e.getStatusCode().value());
reply .setErrorDetails("Failed at reading the source file. " + hcee.getMessage()); reply.setErrorDetails(messageWithCause("Failed at reading the source file", e));
logger.error("Failed to load source file (HttpClientErrorException), sending " +
reply, e);
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
catch (Exception e) catch (Exception e)
{ {
reply.setStatus(INTERNAL_SERVER_ERROR.value()); reply.setStatus(INTERNAL_SERVER_ERROR.value());
reply.setErrorDetails("Failed at reading the source file. " + e.getMessage()); reply.setErrorDetails(messageWithCause("Failed at reading the source file", e));
logger.error("Failed to load source file (Exception), sending " + reply, e);
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
// Create local temp target file in order to run the transformation // Create local temp target file in order to run the transformation
String targetFilename = createTargetFileName(sourceFile.getName(), final String targetFilename = createTargetFileName(sourceFile.getName(),
request.getTargetExtension()); request.getTargetExtension());
File targetFile = buildFile(targetFilename); final File targetFile = buildFile(targetFilename);
// Run the transformation // Run the transformation
try try
{ {
processTransform(sourceFile, targetFile, processTransform(sourceFile, targetFile, request.getTransformRequestOptions(), timeout);
request.getTransformRequestOptions(), timeout);
} }
catch (TransformException te) catch (TransformException e)
{ {
reply.setStatus(te.getStatusCode()); reply.setStatus(e.getStatusCode());
reply.setErrorDetails("Failed at processing transformation. " + te.getMessage()); reply.setErrorDetails(messageWithCause("Failed at processing transformation", e));
logger.error("Failed to perform transform (TransformException), sending " + reply, e);
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
catch (Exception e) catch (Exception e)
{ {
reply.setStatus(INTERNAL_SERVER_ERROR.value()); reply.setStatus(INTERNAL_SERVER_ERROR.value());
reply.setErrorDetails("Failed at processing transformation. " + e.getMessage()); reply.setErrorDetails(messageWithCause("Failed at processing transformation", e));
logger.error("Failed to perform transform (Exception), sending " + reply, e);
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
@ -191,25 +198,29 @@ public abstract class AbstractTransformerController implements TransformControll
{ {
targetRef = alfrescoSharedFileStoreClient.saveFile(targetFile); targetRef = alfrescoSharedFileStoreClient.saveFile(targetFile);
} }
catch (TransformException te) catch (TransformException e)
{ {
reply.setStatus(te.getStatusCode()); reply.setStatus(e.getStatusCode());
reply.setErrorDetails("Failed at writing the transformed file. " + te.getMessage()); reply.setErrorDetails(messageWithCause("Failed at writing the transformed file", e));
logger.error("Failed to save target file (TransformException), sending " + reply, e);
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
catch (HttpClientErrorException hcee) catch (HttpClientErrorException e)
{ {
reply.setStatus(hcee.getStatusCode().value()); reply.setStatus(e.getStatusCode().value());
reply.setErrorDetails("Failed at writing the transformed file. " + hcee.getMessage()); reply.setErrorDetails(messageWithCause("Failed at writing the transformed file. ", e));
logger.error("Failed to save target file (HttpClientErrorException), sending {}" +
reply, e);
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
catch (Exception e) catch (Exception e)
{ {
reply.setStatus(INTERNAL_SERVER_ERROR.value()); reply.setStatus(INTERNAL_SERVER_ERROR.value());
reply.setErrorDetails("Failed at writing the transformed file. " + e.getMessage()); reply.setErrorDetails(messageWithCause("Failed at writing the transformed file. ", e));
logger.error("Failed to save target file (Exception), sending " + reply, e);
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
@ -219,7 +230,8 @@ public abstract class AbstractTransformerController implements TransformControll
} }
catch (Exception e) catch (Exception e)
{ {
logger.error("Failed to delete target local temp file " + targetFile, e); logger.error("Failed to delete local temp target file '{}'. Error will be ignored ",
targetFile, e);
} }
try try
{ {
@ -233,6 +245,7 @@ public abstract class AbstractTransformerController implements TransformControll
reply.setTargetReference(targetRef.getEntry().getFileRef()); reply.setTargetReference(targetRef.getEntry().getFileRef());
reply.setStatus(HttpStatus.CREATED.value()); reply.setStatus(HttpStatus.CREATED.value());
logger.info("Sending successful {}, timeout {} ms", reply);
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
} }
@ -249,7 +262,7 @@ public abstract class AbstractTransformerController implements TransformControll
* @param sourceReference reference to the file in Alfresco Shared File Store * @param sourceReference reference to the file in Alfresco Shared File Store
* @return the file containing the source content for the transformation * @return the file containing the source content for the transformation
*/ */
private File loadSourceFile(final String sourceReference) private File loadSourceFile(final String sourceReference) throws Exception
{ {
ResponseEntity<Resource> responseEntity = alfrescoSharedFileStoreClient ResponseEntity<Resource> responseEntity = alfrescoSharedFileStoreClient
.retrieveFile(sourceReference); .retrieveFile(sourceReference);
@ -262,17 +275,36 @@ public abstract class AbstractTransformerController implements TransformControll
MediaType contentType = headers.getContentType(); MediaType contentType = headers.getContentType();
long size = headers.getContentLength(); long size = headers.getContentLength();
Resource body = responseEntity.getBody(); final Resource body = responseEntity.getBody();
if (body == null)
{
throw new Exception("Failed to retrieve the file body from the request");
}
File file = TempFileProvider.createTempFile("source_", "." + extension); File file = TempFileProvider.createTempFile("source_", "." + extension);
if (logger.isDebugEnabled()) logger.debug("Read source content {} length={} contentType={}",
{ sourceReference, size, contentType);
logger.debug(
"Read source content " + sourceReference + " length="
+ size + " contentType=" + contentType);
}
save(body, file); save(body, file);
LogEntry.setSource(filename, size); LogEntry.setSource(filename, size);
return file; return file;
} }
private static String messageWithCause(final String prefix, Throwable e)
{
final StringBuilder sb = new StringBuilder();
sb.append(prefix).append(" - ")
.append(e.getClass().getSimpleName()).append(": ")
.append(e.getMessage());
while (e.getCause() != null)
{
e = e.getCause();
sb.append(", cause ")
.append(e.getClass().getSimpleName()).append(": ")
.append(e.getMessage());
}
return sb.toString();
}
} }

View File

@ -1,5 +1,6 @@
package org.alfresco.transformer; package org.alfresco.transformer;
import static java.text.MessageFormat.format;
import static org.springframework.http.HttpStatus.BAD_REQUEST; import static org.springframework.http.HttpStatus.BAD_REQUEST;
import java.io.File; import java.io.File;
@ -15,8 +16,8 @@ import org.alfresco.transform.client.model.TransformRequest;
import org.alfresco.transformer.exceptions.TransformException; import org.alfresco.transformer.exceptions.TransformException;
import org.alfresco.transformer.logging.LogEntry; import org.alfresco.transformer.logging.LogEntry;
import org.alfresco.transformer.probes.ProbeTestTransform; import org.alfresco.transformer.probes.ProbeTestTransform;
import org.apache.commons.logging.Log; import org.slf4j.Logger;
import org.apache.commons.logging.LogFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.TypeMismatchException; import org.springframework.beans.TypeMismatchException;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.ui.Model; import org.springframework.ui.Model;
@ -34,7 +35,7 @@ import org.springframework.web.bind.annotation.ResponseBody;
*/ */
public interface TransformController public interface TransformController
{ {
Log logger = LogFactory.getLog(TransformController.class); Logger logger = LoggerFactory.getLogger(TransformController.class);
ResponseEntity<TransformReply> transform(TransformRequest transformRequest, Long timeout); ResponseEntity<TransformReply> transform(TransformRequest transformRequest, Long timeout);
@ -97,49 +98,44 @@ public interface TransformController
default void handleParamsTypeMismatch(HttpServletResponse response, default void handleParamsTypeMismatch(HttpServletResponse response,
MissingServletRequestParameterException e) throws IOException MissingServletRequestParameterException e) throws IOException
{ {
String transformerName = getTransformerName(); final String message = format("Request parameter ''{0}'' is of the wrong type", e
String name = e.getParameterName(); .getParameterName());
String message = "Request parameter " + name + " is of the wrong type"; final int statusCode = BAD_REQUEST.value();
int statusCode = BAD_REQUEST.value();
logger.error(message); logger.error(message, e);
LogEntry.setStatusCodeAndMessage(statusCode, message); LogEntry.setStatusCodeAndMessage(statusCode, message);
response.sendError(statusCode, transformerName + " - " + message); response.sendError(statusCode, getTransformerName() + " - " + message);
} }
@ExceptionHandler(MissingServletRequestParameterException.class) @ExceptionHandler(MissingServletRequestParameterException.class)
default void handleMissingParams(HttpServletResponse response, default void handleMissingParams(HttpServletResponse response,
MissingServletRequestParameterException e) throws IOException MissingServletRequestParameterException e) throws IOException
{ {
String transformerName = getTransformerName(); final String message = format("Request parameter ''{0}'' is missing", e.getParameterName());
String name = e.getParameterName(); final int statusCode = BAD_REQUEST.value();
String message = "Request parameter " + name + " is missing";
int statusCode = BAD_REQUEST.value();
logger.error(message); logger.error(message, e);
LogEntry.setStatusCodeAndMessage(statusCode, message); LogEntry.setStatusCodeAndMessage(statusCode, message);
response.sendError(statusCode, transformerName + " - " + message); response.sendError(statusCode, getTransformerName() + " - " + message);
} }
@ExceptionHandler(TransformException.class) @ExceptionHandler(TransformException.class)
default void transformExceptionWithMessage(HttpServletResponse response, default void transformExceptionWithMessage(HttpServletResponse response,
TransformException e) throws IOException TransformException e) throws IOException
{ {
String transformerName = getTransformerName(); final String message = e.getMessage();
String message = e.getMessage(); final int statusCode = e.getStatusCode();
int statusCode = e.getStatusCode();
logger.error(message); logger.error(message, e);
long time = LogEntry.setStatusCodeAndMessage(statusCode, message); long time = LogEntry.setStatusCodeAndMessage(statusCode, message);
getProbeTestTransform().recordTransformTime(time); getProbeTestTransform().recordTransformTime(time);
// Forced to include the transformer name in the message (see commented out version of this method) response.sendError(statusCode, getTransformerName() + " - " + message);
response.sendError(statusCode, transformerName + " - " + message);
} }
//endregion //endregion
} }

View File

@ -30,8 +30,8 @@ public interface CommandExecutor
run(properties, targetFile, timeout); run(properties, targetFile, timeout);
} }
default void run(String options, File sourceFile, String pageRange, File default void run(String options, File sourceFile, String pageRange, File targetFile,
targetFile, Long timeout) Long timeout)
{ {
LogEntry.setOptions(pageRange + (pageRange.isEmpty() ? "" : " ") + options); LogEntry.setOptions(pageRange + (pageRange.isEmpty() ? "" : " ") + options);

View File

@ -35,8 +35,8 @@ import java.util.Deque;
import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.ConcurrentLinkedDeque;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
import org.apache.commons.logging.Log; import org.slf4j.Logger;
import org.apache.commons.logging.LogFactory; import org.slf4j.LoggerFactory;
/** /**
* Provides setter and getter methods to allow the current Thread to set various log properties and for these * Provides setter and getter methods to allow the current Thread to set various log properties and for these
@ -46,7 +46,7 @@ import org.apache.commons.logging.LogFactory;
*/ */
public final class LogEntry public final class LogEntry
{ {
private static final Log logger = LogFactory.getLog(LogEntry.class); private static final Logger logger = LoggerFactory.getLogger(LogEntry.class);
// TODO allow ProbeTestTransform to find out if there are any transforms running longer than the max time. // TODO allow ProbeTestTransform to find out if there are any transforms running longer than the max time.
private static final AtomicInteger count = new AtomicInteger(0); private static final AtomicInteger count = new AtomicInteger(0);

View File

@ -46,7 +46,8 @@ import org.alfresco.transformer.AbstractTransformerController;
import org.alfresco.transformer.exceptions.TransformException; import org.alfresco.transformer.exceptions.TransformException;
import org.alfresco.transformer.logging.LogEntry; import org.alfresco.transformer.logging.LogEntry;
import org.alfresco.util.TempFileProvider; import org.alfresco.util.TempFileProvider;
import org.apache.commons.logging.Log; import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** /**
* Provides the logic performing test transformations by the live and ready probes. * Provides the logic performing test transformations by the live and ready probes.
@ -72,14 +73,14 @@ import org.apache.commons.logging.Log;
*/ */
public abstract class ProbeTestTransform public abstract class ProbeTestTransform
{ {
private final Logger logger = LoggerFactory.getLogger(ProbeTestTransform.class);
private static final int AVERAGE_OVER_TRANSFORMS = 5; private static final int AVERAGE_OVER_TRANSFORMS = 5;
private final String sourceFilename; private final String sourceFilename;
private final String targetFilename; private final String targetFilename;
private final long minExpectedLength; private final long minExpectedLength;
private final long maxExpectedLength; private final long maxExpectedLength;
private final Log logger;
private int livenessPercent; private int livenessPercent;
private long probeCount; private long probeCount;
private int transCount; private int transCount;
@ -117,13 +118,11 @@ public abstract class ProbeTestTransform
* @param maxTransformSeconds default values normally supplied by helm. Not identical so we can be sure which value is used. * @param maxTransformSeconds default values normally supplied by helm. Not identical so we can be sure which value is used.
* @param livenessTransformPeriodSeconds default values normally supplied by helm. Not identical so we can be sure which value is used. * @param livenessTransformPeriodSeconds default values normally supplied by helm. Not identical so we can be sure which value is used.
*/ */
public ProbeTestTransform(AbstractTransformerController controller, Log logger, public ProbeTestTransform(AbstractTransformerController controller,
String sourceFilename, String targetFilename, long expectedLength, long plusOrMinus, String sourceFilename, String targetFilename, long expectedLength, long plusOrMinus,
int livenessPercent, long maxTransforms, long maxTransformSeconds, int livenessPercent, long maxTransforms, long maxTransformSeconds,
long livenessTransformPeriodSeconds) long livenessTransformPeriodSeconds)
{ {
this.logger = logger;
this.sourceFilename = sourceFilename; this.sourceFilename = sourceFilename;
this.targetFilename = targetFilename; this.targetFilename = targetFilename;
minExpectedLength = Math.max(0, expectedLength-plusOrMinus); minExpectedLength = Math.max(0, expectedLength-plusOrMinus);

View File

@ -95,7 +95,7 @@ public abstract class AbstractHttpRequestTest
private void assertMissingParameter(String name) private void assertMissingParameter(String name)
{ {
assertTransformError(true, assertTransformError(true,
getTransformerName() + " - Request parameter " + name + " is missing"); getTransformerName() + " - Request parameter '" + name + "' is missing");
} }
private void assertTransformError(boolean addFile, String errorMessage) private void assertTransformError(boolean addFile, String errorMessage)

View File

@ -237,7 +237,7 @@ public abstract class AbstractTransformerControllerTest
{ {
mockMvc.perform(mockMvcRequest("/transform", sourceFile)) mockMvc.perform(mockMvcRequest("/transform", sourceFile))
.andExpect(status().is(BAD_REQUEST.value())) .andExpect(status().is(BAD_REQUEST.value()))
.andExpect(status().reason(containsString("Request parameter targetExtension is missing"))); .andExpect(status().reason(containsString("Request parameter 'targetExtension' is missing")));
} }
@Test @Test

16
pom.xml
View File

@ -23,7 +23,6 @@
<dependency.alfresco-core.version>7.3</dependency.alfresco-core.version> <dependency.alfresco-core.version>7.3</dependency.alfresco-core.version>
<dependency.alfresco-data-model.version>8.8</dependency.alfresco-data-model.version> <dependency.alfresco-data-model.version>8.8</dependency.alfresco-data-model.version>
<dependency.alfresco-jodconverter-core.version>3.0.1.1</dependency.alfresco-jodconverter-core.version> <dependency.alfresco-jodconverter-core.version>3.0.1.1</dependency.alfresco-jodconverter-core.version>
<dependency.ch-qos-logback.version>1.2.3</dependency.ch-qos-logback.version>
<env.project_version>${project.version}</env.project_version> <env.project_version>${project.version}</env.project_version>
<alfresco-transform-data-model.version>0.0.7</alfresco-transform-data-model.version> <alfresco-transform-data-model.version>0.0.7</alfresco-transform-data-model.version>
</properties> </properties>
@ -45,21 +44,6 @@
<dependencyManagement> <dependencyManagement>
<dependencies> <dependencies>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-core</artifactId>
<version>${dependency.ch-qos-logback.version}</version>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-access</artifactId>
<version>${dependency.ch-qos-logback.version}</version>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>${dependency.ch-qos-logback.version}</version>
</dependency>
<dependency> <dependency>
<groupId>org.alfresco</groupId> <groupId>org.alfresco</groupId>
<artifactId>alfresco-core</artifactId> <artifactId>alfresco-core</artifactId>