Skip to content

Commit 81b4ea1

Browse files
artembilangaryrussell
artembilan
authored and
garyrussell
committed
Sonar fixes according latest report (#2676)
* Sonar fixes according latest report * Fix initialization order in the `GatewayMethodInboundMessageMapper` * Fix mock in the `DelegatingSessionFactoryTests` * Fix `AbstractRemoteFileOutboundGateway.listFilesInRemoteDir` * * PR comments * * Fix `AbstractRemoteFileOutboundGateway.listFilesInRemoteDir` complexity
1 parent 1943c15 commit 81b4ea1

File tree

9 files changed

+214
-164
lines changed

9 files changed

+214
-164
lines changed

‎spring-integration-core/src/main/java/org/springframework/integration/codec/kryo/AbstractKryoRegistrar.java‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616

1717
package org.springframework.integration.codec.kryo;
1818

19-
import java.util.List;
20-
2119
import org.apache.commons.logging.Log;
2220
import org.apache.commons.logging.LogFactory;
2321

@@ -34,9 +32,9 @@
3432
*/
3533
public abstract class AbstractKryoRegistrar implements KryoRegistrar {
3634

37-
protected static final Kryo kryo = new Kryo();
35+
protected static final Kryo kryo = new Kryo(); // NOSONAR TODO uppercase in 5.2
3836

39-
protected final Log log = LogFactory.getLog(this.getClass());
37+
protected final Log log = LogFactory.getLog(getClass());
4038

4139
@Override
4240
public void registerTypes(Kryo kryo) {
@@ -45,19 +43,13 @@ public void registerTypes(Kryo kryo) {
4543
}
4644
}
4745

48-
/**
49-
* Subclasses implement this to get provided registrations.
50-
* @return a list of {@link Registration}
51-
*/
52-
public abstract List<Registration> getRegistrations();
53-
5446
private void register(Kryo kryo, Registration registration) {
5547
int id = registration.getId();
5648

5749
Registration existing = kryo.getRegistration(id);
5850

5951
if (existing != null) {
60-
throw new RuntimeException("registration already exists " + existing);
52+
throw new IllegalStateException("registration already exists " + existing);
6153
}
6254

6355
if (this.log.isInfoEnabled()) {

‎spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayMethodInboundMessageMapper.java‎

Lines changed: 101 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.lang.annotation.Annotation;
2020
import java.lang.reflect.Method;
21+
import java.util.Arrays;
2122
import java.util.HashMap;
2223
import java.util.LinkedList;
2324
import java.util.List;
@@ -84,7 +85,7 @@
8485
*/
8586
class GatewayMethodInboundMessageMapper implements InboundMessageMapper<Object[]>, BeanFactoryAware {
8687

87-
private static final Log logger = LogFactory.getLog(GatewayMethodInboundMessageMapper.class);
88+
private static final Log LOGGER = LogFactory.getLog(GatewayMethodInboundMessageMapper.class);
8889

8990
private static final SpelExpressionParser PARSER = new SpelExpressionParser();
9091

@@ -145,18 +146,18 @@ class GatewayMethodInboundMessageMapper implements InboundMessageMapper<Object[]
145146
this.globalHeaderExpressions = globalHeaderExpressions;
146147
this.parameterList = getMethodParameterList(method);
147148
this.payloadExpression = parsePayloadExpression(method);
148-
if (mapper == null) {
149-
this.argsMapper = new DefaultMethodArgsMessageMapper();
150-
}
151-
else {
152-
this.argsMapper = mapper;
153-
}
154149
if (messageBuilderFactory == null) {
155150
this.messageBuilderFactory = new DefaultMessageBuilderFactory();
156151
}
157152
else {
158153
this.messageBuilderFactory = messageBuilderFactory;
159154
}
155+
if (mapper == null) {
156+
this.argsMapper = new DefaultMethodArgsMessageMapper();
157+
}
158+
else {
159+
this.argsMapper = mapper;
160+
}
160161
}
161162

162163

@@ -194,13 +195,11 @@ private Message<?> mapArgumentsToMessage(Object[] arguments, @Nullable Map<Strin
194195
try {
195196
return this.argsMapper.toMessage(new MethodArgsHolder(this.method, arguments), headers);
196197
}
198+
catch (MessagingException e) { // NOSONAR fto avoid if..else
199+
throw e;
200+
}
197201
catch (Exception e) {
198-
if (e instanceof MessagingException) {
199-
throw (MessagingException) e;
200-
}
201-
else {
202-
throw new MessageMappingException("Failed to map arguments", e);
203-
}
202+
throw new MessageMappingException("Failed to map arguments: " + Arrays.toString(arguments), e);
204203
}
205204
}
206205

@@ -235,8 +234,8 @@ private void copyHeaders(Map<?, ?> argumentValue, Map<String, Object> headers) {
235234
for (Entry<?, ?> entry : argumentValue.entrySet()) {
236235
Object key = entry.getKey();
237236
if (!(key instanceof String)) {
238-
if (logger.isWarnEnabled()) {
239-
logger.warn("Invalid header name [" + key +
237+
if (LOGGER.isWarnEnabled()) {
238+
LOGGER.warn("Invalid header name [" + key +
240239
"], name type must be String. Skipping mapping of this header to MessageHeaders.");
241240
}
242241
}
@@ -286,13 +285,16 @@ private static Expression parsePayloadExpression(Method method) {
286285

287286
public class DefaultMethodArgsMessageMapper implements MethodArgsMessageMapper {
288287

288+
private final MessageBuilderFactory messageBuilderFactory =
289+
GatewayMethodInboundMessageMapper.this.messageBuilderFactory;
290+
289291
@Override
290292
public Message<?> toMessage(MethodArgsHolder holder, @Nullable Map<String, Object> headers) {
291293
Object messageOrPayload = null;
292294
boolean foundPayloadAnnotation = false;
293295
Object[] arguments = holder.getArgs();
294296
EvaluationContext methodInvocationEvaluationContext = createMethodInvocationEvaluationContext(arguments);
295-
headers =
297+
Map<String, Object> headersToPopulate =
296298
headers != null
297299
? new HashMap<>(headers)
298300
: new HashMap<>();
@@ -309,76 +311,114 @@ public Message<?> toMessage(MethodArgsHolder holder, @Nullable Map<String, Objec
309311
false);
310312
if (annotation != null) {
311313
if (annotation.annotationType().equals(Payload.class)) {
312-
if (messageOrPayload != null) {
313-
throwExceptionForMultipleMessageOrPayloadParameters(methodParameter);
314-
}
315-
String expression = (String) AnnotationUtils.getValue(annotation);
316-
if (!StringUtils.hasText(expression)) {
317-
messageOrPayload = argumentValue;
318-
}
319-
else {
320-
messageOrPayload = evaluatePayloadExpression(expression, argumentValue);
321-
}
314+
messageOrPayload =
315+
processPayloadAnnotation(messageOrPayload, argumentValue, methodParameter, annotation);
322316
foundPayloadAnnotation = true;
323317
}
324318
else if (annotation.annotationType().equals(Header.class)) {
325-
String headerName = determineHeaderName(annotation, methodParameter);
326-
if ((Boolean) AnnotationUtils.getValue(annotation, "required") // NOSONAR never null
327-
&& argumentValue == null) {
328-
throw new IllegalArgumentException("Received null argument value for required header: '"
329-
+ headerName + "'");
330-
}
331-
headers.put(headerName, argumentValue);
319+
processHeaderAnnotation(headersToPopulate, argumentValue, methodParameter, annotation);
332320
}
333321
else if (annotation.annotationType().equals(Headers.class)) {
334-
if (argumentValue != null) {
335-
if (!(argumentValue instanceof Map)) {
336-
throw new IllegalArgumentException(
337-
"@Headers annotation is only valid for Map-typed parameters");
338-
}
339-
for (Object key : ((Map<?, ?>) argumentValue).keySet()) {
340-
Assert.isInstanceOf(String.class, key, "Invalid header name [" + key +
341-
"], name type must be String.");
342-
Object value = ((Map<?, ?>) argumentValue).get(key);
343-
headers.put((String) key, value);
344-
}
345-
}
322+
processHeadersAnnotation(headersToPopulate, argumentValue);
346323
}
347324
}
348325
else if (messageOrPayload == null) {
349326
messageOrPayload = argumentValue;
350327
}
351328
else if (Map.class.isAssignableFrom(methodParameter.getParameterType())) {
352-
if (messageOrPayload instanceof Map && !foundPayloadAnnotation) {
353-
if (GatewayMethodInboundMessageMapper.this.payloadExpression == null) {
354-
throw new MessagingException("Ambiguous method parameters; found more than one " +
355-
"Map-typed parameter and neither one contains a @Payload annotation");
356-
}
357-
}
358-
GatewayMethodInboundMessageMapper.this.copyHeaders((Map<?, ?>) argumentValue, headers);
329+
processMapArgument(messageOrPayload, foundPayloadAnnotation, headersToPopulate,
330+
(Map<?, ?>) argumentValue);
359331
}
360332
else if (GatewayMethodInboundMessageMapper.this.payloadExpression == null) {
361-
GatewayMethodInboundMessageMapper.this
362-
.throwExceptionForMultipleMessageOrPayloadParameters(methodParameter);
333+
throwExceptionForMultipleMessageOrPayloadParameters(methodParameter);
363334
}
364335
}
365-
Assert.isTrue(messageOrPayload != null, "unable to determine a Message or payload parameter on method ["
366-
+ GatewayMethodInboundMessageMapper.this.method + "]");
336+
337+
Assert.isTrue(messageOrPayload != null,
338+
() -> "unable to determine a Message or payload parameter on method ["
339+
+ GatewayMethodInboundMessageMapper.this.method + "]");
340+
populateSendAndReplyTimeoutHeaders(methodInvocationEvaluationContext, headersToPopulate);
341+
342+
return buildMessage(headersToPopulate, messageOrPayload, methodInvocationEvaluationContext);
343+
}
344+
345+
@Nullable
346+
private Object processPayloadAnnotation(@Nullable Object messageOrPayload,
347+
Object argumentValue, MethodParameter methodParameter, Annotation annotation) {
348+
349+
if (messageOrPayload != null) {
350+
throwExceptionForMultipleMessageOrPayloadParameters(methodParameter);
351+
}
352+
String expression = (String) AnnotationUtils.getValue(annotation);
353+
if (!StringUtils.hasText(expression)) {
354+
return argumentValue;
355+
}
356+
else {
357+
return evaluatePayloadExpression(expression, argumentValue);
358+
}
359+
}
360+
361+
private void processHeaderAnnotation(Map<String, Object> headersToPopulate, @Nullable Object argumentValue,
362+
MethodParameter methodParameter, Annotation annotation) {
363+
364+
String headerName = determineHeaderName(annotation, methodParameter);
365+
if ((Boolean) AnnotationUtils.getValue(annotation, "required") // NOSONAR never null
366+
&& argumentValue == null) {
367+
throw new IllegalArgumentException("Received null argument value for required header: '"
368+
+ headerName + "'");
369+
}
370+
headersToPopulate.put(headerName, argumentValue);
371+
}
372+
373+
private void processHeadersAnnotation(Map<String, Object> headersToPopulate, @Nullable Object argumentValue) {
374+
if (argumentValue != null) {
375+
if (!(argumentValue instanceof Map)) {
376+
throw new IllegalArgumentException(
377+
"@Headers annotation is only valid for Map-typed parameters");
378+
}
379+
for (Object key : ((Map<?, ?>) argumentValue).keySet()) {
380+
Assert.isInstanceOf(String.class, key, "Invalid header name [" + key +
381+
"], name type must be String.");
382+
Object value = ((Map<?, ?>) argumentValue).get(key);
383+
headersToPopulate.put((String) key, value);
384+
}
385+
}
386+
}
387+
388+
private void processMapArgument(Object messageOrPayload, boolean foundPayloadAnnotation,
389+
Map<String, Object> headersToPopulate, Map<?, ?> argumentValue) {
390+
391+
if (messageOrPayload instanceof Map && !foundPayloadAnnotation) {
392+
if (GatewayMethodInboundMessageMapper.this.payloadExpression == null) {
393+
throw new MessagingException("Ambiguous method parameters; found more than one " +
394+
"Map-typed parameter and neither one contains a @Payload annotation");
395+
}
396+
}
397+
copyHeaders(argumentValue, headersToPopulate);
398+
}
399+
400+
private void populateSendAndReplyTimeoutHeaders(EvaluationContext methodInvocationEvaluationContext,
401+
Map<String, Object> headersToPopulate) {
402+
367403
if (GatewayMethodInboundMessageMapper.this.sendTimeoutExpression != null) {
368-
headers.computeIfAbsent(GenericMessagingTemplate.DEFAULT_SEND_TIMEOUT_HEADER,
404+
headersToPopulate.computeIfAbsent(GenericMessagingTemplate.DEFAULT_SEND_TIMEOUT_HEADER,
369405
v -> GatewayMethodInboundMessageMapper.this.sendTimeoutExpression
370406
.getValue(methodInvocationEvaluationContext, Long.class));
371407
}
372408
if (GatewayMethodInboundMessageMapper.this.replyTimeoutExpression != null) {
373-
headers.computeIfAbsent(GenericMessagingTemplate.DEFAULT_RECEIVE_TIMEOUT_HEADER,
409+
headersToPopulate.computeIfAbsent(GenericMessagingTemplate.DEFAULT_RECEIVE_TIMEOUT_HEADER,
374410
v -> GatewayMethodInboundMessageMapper.this.replyTimeoutExpression
375411
.getValue(methodInvocationEvaluationContext, Long.class));
376412
}
377-
MessageBuilderFactory messageBuilderFactory = GatewayMethodInboundMessageMapper.this.messageBuilderFactory;
413+
}
414+
415+
private Message<?> buildMessage(Map<String, Object> headers, Object messageOrPayload,
416+
EvaluationContext methodInvocationEvaluationContext) {
417+
378418
AbstractIntegrationMessageBuilder<?> builder =
379419
(messageOrPayload instanceof Message)
380-
? messageBuilderFactory.fromMessage((Message<?>) messageOrPayload)
381-
: messageBuilderFactory.withPayload(messageOrPayload);
420+
? this.messageBuilderFactory.fromMessage((Message<?>) messageOrPayload)
421+
: this.messageBuilderFactory.withPayload(messageOrPayload);
382422
builder.copyHeadersIfAbsent(headers);
383423
// Explicit headers in XML override any @Header annotations...
384424
if (!CollectionUtils.isEmpty(GatewayMethodInboundMessageMapper.this.headerExpressions)) {

‎spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java‎

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -743,13 +743,13 @@ else if (this.options.contains(Option.RECURSIVE)) {
743743
catch (Exception e) {
744744
if (replies.size() > 0) {
745745
throw new PartialSuccessException(requestMessage,
746-
"Partially successful 'mput' operation" + (subDirectory == null ? "" : (" on " + subDirectory)),
747-
e, replies, filteredFiles);
746+
"Partially successful 'mput' operation" +
747+
(subDirectory == null ? "" : (" on " + subDirectory)), e, replies, filteredFiles);
748748
}
749749
else if (e instanceof PartialSuccessException) {
750750
throw new PartialSuccessException(requestMessage,
751-
"Partially successful 'mput' operation" + (subDirectory == null ? "" : (" on " + subDirectory)),
752-
e, replies, filteredFiles);
751+
"Partially successful 'mput' operation" +
752+
(subDirectory == null ? "" : (" on " + subDirectory)), e, replies, filteredFiles);
753753
}
754754
else {
755755
throw e;
@@ -801,28 +801,15 @@ protected List<?> ls(Message<?> message, Session<F> session, String dir) throws
801801
private List<F> listFilesInRemoteDir(Session<F> session, String directory, String subDirectory)
802802
throws IOException {
803803

804-
List<F> lsFiles = new ArrayList<F>();
804+
List<F> lsFiles = new ArrayList<>();
805805
String remoteDirectory = buildRemotePath(directory, subDirectory);
806806

807807
F[] files = session.list(remoteDirectory);
808808
boolean recursion = this.options.contains(Option.RECURSIVE);
809809
if (!ObjectUtils.isEmpty(files)) {
810-
Collection<F> filteredFiles = this.filterFiles(files);
811-
for (F file : filteredFiles) {
812-
String fileName = this.getFilename(file);
810+
for (F file : filterFiles(files)) {
813811
if (file != null) {
814-
if (this.options.contains(Option.SUBDIRS) || !this.isDirectory(file)) {
815-
if (recursion && StringUtils.hasText(subDirectory)) {
816-
lsFiles.add(enhanceNameWithSubDirectory(file, subDirectory));
817-
}
818-
else {
819-
lsFiles.add(file);
820-
}
821-
}
822-
if (recursion && this.isDirectory(file) && !(".".equals(fileName)) && !("..".equals(fileName))) {
823-
lsFiles.addAll(listFilesInRemoteDir(session, directory, subDirectory + fileName
824-
+ this.remoteFileTemplate.getRemoteFileSeparator()));
825-
}
812+
processFile(session, directory, subDirectory, lsFiles, recursion, file);
826813
}
827814
}
828815
}
@@ -844,6 +831,24 @@ protected final List<F> filterFiles(F[] files) {
844831
return (this.filter != null) ? this.filter.filterFiles(files) : Arrays.asList(files);
845832
}
846833

834+
private void processFile(Session<F> session, String directory, String subDirectory, List<F> lsFiles,
835+
boolean recursion, F file) throws IOException {
836+
837+
if (this.options.contains(Option.SUBDIRS) || !isDirectory(file)) {
838+
if (recursion && StringUtils.hasText(subDirectory)) {
839+
lsFiles.add(enhanceNameWithSubDirectory(file, subDirectory));
840+
}
841+
else {
842+
lsFiles.add(file);
843+
}
844+
}
845+
String fileName = getFilename(file);
846+
if (recursion && isDirectory(file) && !(".".equals(fileName)) && !("..".equals(fileName))) {
847+
lsFiles.addAll(listFilesInRemoteDir(session, directory,
848+
subDirectory + fileName + this.remoteFileTemplate.getRemoteFileSeparator()));
849+
}
850+
}
851+
847852
protected final List<File> filterMputFiles(File[] files) {
848853
if (files == null) {
849854
return Collections.emptyList();
@@ -1009,7 +1014,8 @@ private List<File> mGetWithoutRecursion(Message<?> message, Session<F> session,
10091014
*/
10101015
String fileName = this.getRemoteFilename(fullFileName);
10111016
String actualRemoteDirectory = this.getRemoteDirectory(fullFileName, fileName);
1012-
File file = get(message, session, actualRemoteDirectory, fullFileName, fileName, lsEntry.getFileInfo());
1017+
File file = get(message, session, actualRemoteDirectory, fullFileName, fileName,
1018+
lsEntry.getFileInfo());
10131019
if (file != null) {
10141020
files.add(file);
10151021
}
@@ -1044,16 +1050,18 @@ private List<File> mGetWithRecursion(Message<?> message, Session<F> session, Str
10441050
}
10451051
try {
10461052
for (AbstractFileInfo<F> lsEntry : fileNames) {
1047-
String fullFileName = remoteDirectory != null
1048-
? remoteDirectory + getFilename(lsEntry)
1049-
: getFilename(lsEntry);
1053+
String fullFileName =
1054+
remoteDirectory != null
1055+
? remoteDirectory + getFilename(lsEntry)
1056+
: getFilename(lsEntry);
10501057
/*
10511058
* With recursion, the filename might contain subdirectory information
10521059
* normalize each file separately.
10531060
*/
10541061
String fileName = this.getRemoteFilename(fullFileName);
10551062
String actualRemoteDirectory = this.getRemoteDirectory(fullFileName, fileName);
1056-
File file = get(message, session, actualRemoteDirectory, fullFileName, fileName, lsEntry.getFileInfo());
1063+
File file = get(message, session, actualRemoteDirectory, fullFileName, fileName,
1064+
lsEntry.getFileInfo());
10571065
if (file != null) {
10581066
files.add(file);
10591067
}

0 commit comments

Comments
 (0)