mirror of
https://github.com/Alfresco/alfresco-transform-core.git
synced 2025-07-31 17:38:33 +00:00
ATS-532 : Code improvements (#89)
- move startup message from controllers to the Application classes (SpringBoot configuration beans) - added static imports for most static variables and static methods - simplified a few nested *if*s - replaced Arrays.asList() with explicit immutable collections - fixed a few IntelliJ code inspection warnings
This commit is contained in:
@@ -33,9 +33,11 @@ import static org.alfresco.transformer.fs.FileManager.deleteFile;
|
||||
import static org.alfresco.transformer.fs.FileManager.getFilenameFromContentDisposition;
|
||||
import static org.alfresco.transformer.fs.FileManager.save;
|
||||
import static org.springframework.http.HttpStatus.BAD_REQUEST;
|
||||
import static org.springframework.http.HttpStatus.CREATED;
|
||||
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
import static org.springframework.http.HttpStatus.OK;
|
||||
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
|
||||
import static org.springframework.util.StringUtils.getFilenameExtension;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
@@ -59,7 +61,6 @@ import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.validation.DirectFieldBindingResult;
|
||||
import org.springframework.validation.Errors;
|
||||
import org.springframework.web.bind.annotation.GetMapping;
|
||||
@@ -162,7 +163,7 @@ public abstract class AbstractTransformerController implements TransformControll
|
||||
final Errors errors = validateTransformRequest(request);
|
||||
if (!errors.getAllErrors().isEmpty())
|
||||
{
|
||||
reply.setStatus(HttpStatus.BAD_REQUEST.value());
|
||||
reply.setStatus(BAD_REQUEST.value());
|
||||
reply.setErrorDetails(errors
|
||||
.getAllErrors()
|
||||
.stream()
|
||||
@@ -251,8 +252,8 @@ public abstract class AbstractTransformerController implements TransformControll
|
||||
reply.setStatus(e.getStatusCode().value());
|
||||
reply.setErrorDetails(messageWithCause("Failed at writing the transformed file. ", e));
|
||||
|
||||
logger.error("Failed to save target file (HttpClientErrorException), sending " +
|
||||
reply, e);
|
||||
logger.error("Failed to save target file (HttpClientErrorException), sending " + reply,
|
||||
e);
|
||||
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
|
||||
}
|
||||
catch (Exception e)
|
||||
@@ -283,9 +284,9 @@ public abstract class AbstractTransformerController implements TransformControll
|
||||
}
|
||||
|
||||
reply.setTargetReference(targetRef.getEntry().getFileRef());
|
||||
reply.setStatus(HttpStatus.CREATED.value());
|
||||
reply.setStatus(CREATED.value());
|
||||
|
||||
logger.info("Sending successful {}, timeout {} ms", reply);
|
||||
logger.info("Sending successful {}, timeout {} ms", reply, timeout);
|
||||
return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus()));
|
||||
}
|
||||
|
||||
@@ -302,7 +303,7 @@ public abstract class AbstractTransformerController implements TransformControll
|
||||
* @param sourceReference reference to the file in Alfresco Shared File Store
|
||||
* @return the file containing the source content for the transformation
|
||||
*/
|
||||
private File loadSourceFile(final String sourceReference) throws Exception
|
||||
private File loadSourceFile(final String sourceReference)
|
||||
{
|
||||
ResponseEntity<Resource> responseEntity = alfrescoSharedFileStoreClient
|
||||
.retrieveFile(sourceReference);
|
||||
@@ -311,7 +312,7 @@ public abstract class AbstractTransformerController implements TransformControll
|
||||
HttpHeaders headers = responseEntity.getHeaders();
|
||||
String filename = getFilenameFromContentDisposition(headers);
|
||||
|
||||
String extension = StringUtils.getFilenameExtension(filename);
|
||||
String extension = getFilenameExtension(filename);
|
||||
MediaType contentType = headers.getContentType();
|
||||
long size = headers.getContentLength();
|
||||
|
||||
|
@@ -26,6 +26,9 @@
|
||||
*/
|
||||
package org.alfresco.transformer;
|
||||
|
||||
import static org.springframework.http.HttpStatus.BAD_REQUEST;
|
||||
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
|
||||
import java.util.Optional;
|
||||
|
||||
import javax.jms.Destination;
|
||||
@@ -152,14 +155,14 @@ public class QueueTransformService
|
||||
String message =
|
||||
"MessageConversionException during T-Request deserialization of message with correlationID "
|
||||
+ correlationId + ": ";
|
||||
throw new TransformException(HttpStatus.BAD_REQUEST.value(), message + e.getMessage());
|
||||
throw new TransformException(BAD_REQUEST.value(), message + e.getMessage());
|
||||
}
|
||||
catch (JMSException e)
|
||||
{
|
||||
String message =
|
||||
"JMSException during T-Request deserialization of message with correlationID "
|
||||
+ correlationId + ": ";
|
||||
throw new TransformException(HttpStatus.INTERNAL_SERVER_ERROR.value(),
|
||||
throw new TransformException(INTERNAL_SERVER_ERROR.value(),
|
||||
message + e.getMessage());
|
||||
}
|
||||
catch (Exception e)
|
||||
@@ -167,7 +170,7 @@ public class QueueTransformService
|
||||
String message =
|
||||
"Exception during T-Request deserialization of message with correlationID "
|
||||
+ correlationId + ": ";
|
||||
throw new TransformException(HttpStatus.INTERNAL_SERVER_ERROR.value(),
|
||||
throw new TransformException(INTERNAL_SERVER_ERROR.value(),
|
||||
message + e.getMessage());
|
||||
}
|
||||
}
|
||||
@@ -175,7 +178,7 @@ public class QueueTransformService
|
||||
private void replyWithInternalSvErr(final Destination destination, final String msg,
|
||||
final String correlationId)
|
||||
{
|
||||
replyWithError(destination, HttpStatus.INTERNAL_SERVER_ERROR, msg, correlationId);
|
||||
replyWithError(destination, INTERNAL_SERVER_ERROR, msg, correlationId);
|
||||
}
|
||||
|
||||
private void replyWithError(final Destination destination, final HttpStatus status,
|
||||
|
@@ -26,6 +26,9 @@
|
||||
*/
|
||||
package org.alfresco.transformer.clients;
|
||||
|
||||
import static org.springframework.http.HttpMethod.POST;
|
||||
import static org.springframework.http.MediaType.MULTIPART_FORM_DATA;
|
||||
|
||||
import java.io.File;
|
||||
|
||||
import org.alfresco.transform.exceptions.TransformException;
|
||||
@@ -36,8 +39,6 @@ import org.springframework.core.io.FileSystemResource;
|
||||
import org.springframework.core.io.Resource;
|
||||
import org.springframework.http.HttpEntity;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.HttpMethod;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.util.LinkedMultiValueMap;
|
||||
import org.springframework.web.client.HttpClientErrorException;
|
||||
@@ -87,11 +88,11 @@ public class AlfrescoSharedFileStoreClient
|
||||
LinkedMultiValueMap<String, Object> map = new LinkedMultiValueMap<>();
|
||||
map.add("file", value);
|
||||
HttpHeaders headers = new HttpHeaders();
|
||||
headers.setContentType(MediaType.MULTIPART_FORM_DATA);
|
||||
headers.setContentType(MULTIPART_FORM_DATA);
|
||||
HttpEntity<LinkedMultiValueMap<String, Object>> requestEntity = new HttpEntity<>(map,
|
||||
headers);
|
||||
ResponseEntity<FileRefResponse> responseEntity = restTemplate
|
||||
.exchange(fileStoreUrl, HttpMethod.POST, requestEntity, FileRefResponse.class);
|
||||
.exchange(fileStoreUrl, POST, requestEntity, FileRefResponse.class);
|
||||
return responseEntity.getBody();
|
||||
}
|
||||
catch (HttpClientErrorException e)
|
||||
|
@@ -26,9 +26,12 @@
|
||||
*/
|
||||
package org.alfresco.transformer.fs;
|
||||
|
||||
import static org.springframework.http.HttpHeaders.CONTENT_DISPOSITION;
|
||||
import static org.springframework.http.HttpStatus.BAD_REQUEST;
|
||||
import static org.springframework.http.HttpStatus.INSUFFICIENT_STORAGE;
|
||||
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
import static org.springframework.util.StringUtils.getFilename;
|
||||
import static org.springframework.util.StringUtils.getFilenameExtension;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
@@ -46,7 +49,6 @@ import org.springframework.core.io.Resource;
|
||||
import org.springframework.core.io.UrlResource;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.web.multipart.MultipartFile;
|
||||
import org.springframework.web.util.UriUtils;
|
||||
|
||||
@@ -99,7 +101,7 @@ public class FileManager
|
||||
*/
|
||||
private static String checkFilename(boolean source, String filename)
|
||||
{
|
||||
filename = StringUtils.getFilename(filename);
|
||||
filename = getFilename(filename);
|
||||
if (filename == null || filename.isEmpty())
|
||||
{
|
||||
String sourceOrTarget = source ? "source" : "target";
|
||||
@@ -162,7 +164,7 @@ public class FileManager
|
||||
public static String getFilenameFromContentDisposition(HttpHeaders headers)
|
||||
{
|
||||
String filename = "";
|
||||
String contentDisposition = headers.getFirst(HttpHeaders.CONTENT_DISPOSITION);
|
||||
String contentDisposition = headers.getFirst(CONTENT_DISPOSITION);
|
||||
if (contentDisposition != null)
|
||||
{
|
||||
String[] strings = contentDisposition.split("; *");
|
||||
@@ -184,14 +186,14 @@ public class FileManager
|
||||
*/
|
||||
public static String createTargetFileName(final String fileName, final String targetExtension)
|
||||
{
|
||||
final String sourceFilename = StringUtils.getFilename(fileName);
|
||||
final String sourceFilename = getFilename(fileName);
|
||||
|
||||
if (sourceFilename == null || sourceFilename.isEmpty())
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
final String ext = StringUtils.getFilenameExtension(sourceFilename);
|
||||
final String ext = getFilenameExtension(sourceFilename);
|
||||
|
||||
if (ext == null || ext.isEmpty())
|
||||
{
|
||||
@@ -235,9 +237,8 @@ public class FileManager
|
||||
targetFile)
|
||||
{
|
||||
Resource targetResource = load(targetFile);
|
||||
targetFilename = UriUtils.encodePath(StringUtils.getFilename(targetFilename), "UTF-8");
|
||||
return ResponseEntity.ok().header(HttpHeaders
|
||||
.CONTENT_DISPOSITION,
|
||||
targetFilename = UriUtils.encodePath(getFilename(targetFilename), "UTF-8");
|
||||
return ResponseEntity.ok().header(CONTENT_DISPOSITION,
|
||||
"attachment; filename*= UTF-8''" + targetFilename).body(targetResource);
|
||||
}
|
||||
}
|
||||
|
@@ -54,25 +54,13 @@ public class FileRefEntity
|
||||
return fileRef;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString()
|
||||
{
|
||||
return fileRef;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean equals(Object o)
|
||||
{
|
||||
if (this == o)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
if (o == null || getClass() != o.getClass())
|
||||
{
|
||||
return false;
|
||||
}
|
||||
FileRefEntity fileRef = (FileRefEntity) o;
|
||||
return Objects.equals(this.fileRef, fileRef.fileRef);
|
||||
if (this == o) return true;
|
||||
if (o == null || getClass() != o.getClass()) return false;
|
||||
FileRefEntity that = (FileRefEntity) o;
|
||||
return Objects.equals(fileRef, that.fileRef);
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -80,4 +68,10 @@ public class FileRefEntity
|
||||
{
|
||||
return Objects.hash(fileRef);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString()
|
||||
{
|
||||
return fileRef;
|
||||
}
|
||||
}
|
||||
|
@@ -35,9 +35,7 @@ public class FileRefResponse
|
||||
{
|
||||
private FileRefEntity entry;
|
||||
|
||||
public FileRefResponse()
|
||||
{
|
||||
}
|
||||
public FileRefResponse() {}
|
||||
|
||||
public FileRefResponse(FileRefEntity entry)
|
||||
{
|
||||
|
@@ -27,6 +27,8 @@
|
||||
package org.alfresco.transformer;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.springframework.http.HttpMethod.POST;
|
||||
import static org.springframework.http.MediaType.MULTIPART_FORM_DATA;
|
||||
import static org.springframework.test.util.AssertionErrors.assertTrue;
|
||||
|
||||
import org.junit.Test;
|
||||
@@ -35,8 +37,6 @@ import org.springframework.boot.test.web.client.TestRestTemplate;
|
||||
import org.springframework.boot.web.server.LocalServerPort;
|
||||
import org.springframework.http.HttpEntity;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.HttpMethod;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.util.LinkedMultiValueMap;
|
||||
|
||||
@@ -115,11 +115,11 @@ public abstract class AbstractHttpRequestTest
|
||||
new org.springframework.core.io.ClassPathResource("quick." + getSourceExtension()));
|
||||
}
|
||||
HttpHeaders headers = new HttpHeaders();
|
||||
headers.setContentType(MediaType.MULTIPART_FORM_DATA);
|
||||
headers.setContentType(MULTIPART_FORM_DATA);
|
||||
HttpEntity<LinkedMultiValueMap<String, Object>> entity = new HttpEntity<>(parameters,
|
||||
headers);
|
||||
ResponseEntity<String> response = restTemplate.exchange("/transform", HttpMethod.POST,
|
||||
entity, String.class, "");
|
||||
ResponseEntity<String> response = restTemplate.exchange("/transform", POST, entity,
|
||||
String.class, "");
|
||||
assertEquals(errorMessage, getErrorMessage(response.getBody()));
|
||||
}
|
||||
|
||||
|
@@ -30,11 +30,13 @@ import static org.hamcrest.Matchers.containsString;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.springframework.http.HttpHeaders.ACCEPT;
|
||||
import static org.springframework.http.HttpHeaders.CONTENT_TYPE;
|
||||
import static org.springframework.http.HttpStatus.BAD_REQUEST;
|
||||
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
import static org.springframework.http.HttpStatus.OK;
|
||||
import static org.springframework.http.MediaType.APPLICATION_JSON_UTF8_VALUE;
|
||||
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
|
||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
|
||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
|
||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
|
||||
@@ -64,8 +66,6 @@ import org.junit.Test;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.test.mock.mockito.MockBean;
|
||||
import org.springframework.core.io.ClassPathResource;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.mock.web.MockMultipartFile;
|
||||
import org.springframework.test.util.ReflectionTestUtils;
|
||||
import org.springframework.test.web.servlet.MockMvc;
|
||||
@@ -314,8 +314,8 @@ public abstract class AbstractTransformerControllerTest
|
||||
String transformationReplyAsString = mockMvc
|
||||
.perform(MockMvcRequestBuilders
|
||||
.post("/transform")
|
||||
.header(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE)
|
||||
.header(CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
|
||||
.header(ACCEPT, APPLICATION_JSON_VALUE)
|
||||
.header(CONTENT_TYPE, APPLICATION_JSON_VALUE)
|
||||
.content(tr))
|
||||
.andExpect(status().is(BAD_REQUEST.value()))
|
||||
.andReturn().getResponse().getContentAsString();
|
||||
@@ -383,10 +383,11 @@ public abstract class AbstractTransformerControllerTest
|
||||
ReflectionTestUtils.setField(AbstractTransformerController.class, "ENGINE_CONFIG",
|
||||
"engine_config_incomplete.json");
|
||||
|
||||
String response = mockMvc.perform(MockMvcRequestBuilders.get("/transform/config"))
|
||||
.andExpect(status().is(OK.value())).andExpect(
|
||||
header().string(CONTENT_TYPE, APPLICATION_JSON_UTF8_VALUE))
|
||||
.andReturn().getResponse().getContentAsString();
|
||||
String response = mockMvc
|
||||
.perform(MockMvcRequestBuilders.get("/transform/config"))
|
||||
.andExpect(status().is(OK.value()))
|
||||
.andExpect(header().string(CONTENT_TYPE, APPLICATION_JSON_UTF8_VALUE))
|
||||
.andReturn().getResponse().getContentAsString();
|
||||
|
||||
TransformConfig transformConfig = objectMapper.readValue(response, TransformConfig.class);
|
||||
|
||||
|
@@ -32,6 +32,9 @@ import static org.mockito.Mockito.doThrow;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.verifyNoMoreInteractions;
|
||||
import static org.springframework.http.HttpStatus.BAD_REQUEST;
|
||||
import static org.springframework.http.HttpStatus.CREATED;
|
||||
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
|
||||
import javax.jms.Destination;
|
||||
import javax.jms.JMSException;
|
||||
@@ -100,7 +103,7 @@ public class QueueTransformServiceTest
|
||||
|
||||
TransformReply reply = TransformReply
|
||||
.builder()
|
||||
.withStatus(HttpStatus.INTERNAL_SERVER_ERROR.value())
|
||||
.withStatus(INTERNAL_SERVER_ERROR.value())
|
||||
.withErrorDetails(
|
||||
"JMS exception during T-Request deserialization of message with correlationID "
|
||||
+ msg.getCorrelationId() + ": null")
|
||||
@@ -127,7 +130,7 @@ public class QueueTransformServiceTest
|
||||
|
||||
TransformReply reply = TransformReply
|
||||
.builder()
|
||||
.withStatus(HttpStatus.BAD_REQUEST.value())
|
||||
.withStatus(BAD_REQUEST.value())
|
||||
.withErrorDetails(
|
||||
"Message conversion exception during T-Request deserialization of message with correlationID"
|
||||
+ msg.getCorrelationId() + ": null")
|
||||
@@ -154,9 +157,10 @@ public class QueueTransformServiceTest
|
||||
|
||||
TransformReply reply = TransformReply
|
||||
.builder()
|
||||
.withStatus(HttpStatus.INTERNAL_SERVER_ERROR.value()).withErrorDetails(
|
||||
"JMSException during T-Request deserialization of message with correlationID " + msg
|
||||
.getCorrelationId() + ": null")
|
||||
.withStatus(INTERNAL_SERVER_ERROR.value())
|
||||
.withErrorDetails(
|
||||
"JMSException during T-Request deserialization of message with correlationID " +
|
||||
msg.getCorrelationId() + ": null")
|
||||
.build();
|
||||
|
||||
doThrow(JMSException.class).when(transformMessageConverter).fromMessage(msg);
|
||||
@@ -179,7 +183,7 @@ public class QueueTransformServiceTest
|
||||
TransformRequest request = new TransformRequest();
|
||||
TransformReply reply = TransformReply
|
||||
.builder()
|
||||
.withStatus(HttpStatus.CREATED.value())
|
||||
.withStatus(CREATED.value())
|
||||
.build();
|
||||
|
||||
doReturn(request).when(transformMessageConverter).fromMessage(msg);
|
||||
@@ -218,9 +222,10 @@ public class QueueTransformServiceTest
|
||||
doReturn(destination).when(msg).getJMSReplyTo();
|
||||
|
||||
TransformRequest request = new TransformRequest();
|
||||
TransformReply reply = TransformReply.builder()
|
||||
.withStatus(HttpStatus.CREATED.value())
|
||||
.build();
|
||||
TransformReply reply = TransformReply
|
||||
.builder()
|
||||
.withStatus(CREATED.value())
|
||||
.build();
|
||||
|
||||
doReturn(request).when(transformMessageConverter).fromMessage(msg);
|
||||
doReturn(new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())))
|
||||
|
Reference in New Issue
Block a user