Polishing - PR Comments

spring-projects · Feb 24, 2017 · 92692de · 92692de
1 parent ea66259
commit 92692de
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 64 deletions.
diff --git a/...gration-core/src/main/java/org/springframework/integration/json/SimpleJsonSerializer.java b/...gration-core/src/main/java/org/springframework/integration/json/SimpleJsonSerializer.java
@@ -16,16 +16,17 @@
 
 package org.springframework.integration.json;
 
+import java.beans.PropertyDescriptor;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.Arrays;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import org.springframework.util.ReflectionUtils;
+import org.springframework.beans.BeanUtils;
 
 /**
  * Extremely simple JSON serializer. Only handles top level
@@ -46,41 +47,41 @@ private SimpleJsonSerializer() {
 	/**
 	 * Convert the bean to JSON with the provided properties.
 	 * @param bean the object to serialize.
-	 * @param properties the property names (must have getters).
+	 * @param propertiesToExclude the property names to ignore.
 	 * @return the JSON.
 	 */
-	public static String toJson(Object bean, String... properties) {
-		Map<String, String> propertyMap = new HashMap<>();
-		List<String> list = Arrays.asList(properties);
-		list.forEach(p -> {
-			char[] chars = p.toCharArray();
-			chars[0] = Character.toUpperCase(chars[0]);
-			propertyMap.put(new String(chars), p);
-		});
+	public static String toJson(Object bean, String... propertiesToExclude) {
+		PropertyDescriptor[] propertyDescriptors = BeanUtils.getPropertyDescriptors(bean.getClass());
+		Set<String> excluded = new HashSet<>(Arrays.asList(propertiesToExclude));
+		excluded.add("class");
 		final StringBuilder stringBuilder = new StringBuilder("{");
 		final Object[] emptyArgs = new Object[0];
-		ReflectionUtils.doWithMethods(bean.getClass(), method -> {
-			int beginIndex = method.getName().startsWith("get") ? 3 : 2;
-			stringBuilder.append(toElement(propertyMap.get(method.getName().substring(beginIndex)))).append(":");
-			Object result;
-			try {
-				result = method.invoke(bean, emptyArgs);
-			}
-			catch (InvocationTargetException e) {
-				if (logger.isDebugEnabled()) {
-					logger.debug("Failed to serialize property " + method.getName(), e);
+		for (PropertyDescriptor descriptor : propertyDescriptors) {
+			String propertyName = descriptor.getName();
+			Method readMethod = descriptor.getReadMethod();
+			if (!excluded.contains(propertyName) && readMethod != null) {
+				stringBuilder.append(toElement(propertyName)).append(":");
+				Object result;
+				try {
+					result = readMethod.invoke(bean, emptyArgs);
+				}
+				catch (InvocationTargetException | IllegalAccessException | IllegalArgumentException e) {
+					if (logger.isDebugEnabled()) {
+						logger.debug("Failed to serialize property " + propertyName, e);
+					}
+					result = e.getMessage();
 				}
-				result = e.getMessage();
+				stringBuilder.append(toElement(result)).append(",");
 			}
-			stringBuilder.append(toElement(result)).append(",");
-		}, method -> {
-			String name = method.getName();
-			return (name.length() > 3 && name.startsWith("get") && propertyMap.keySet().contains(name.substring(3)))
-				|| (name.length() > 2 && name.startsWith("is") && propertyMap.keySet().contains(name.substring(2)));
-		});
+		}
 		stringBuilder.setLength(stringBuilder.length() - 1);
 		stringBuilder.append("}");
-		return stringBuilder.toString();
+		if (stringBuilder.length() == 1) {
+			return null;
+		}
+		else {
+			return stringBuilder.toString();
+		}
 	}
 
 	private static String toElement(Object result) {

diff --git a/...on-core/src/test/java/org/springframework/integration/json/SimpleJsonSerializerTests.java b/...on-core/src/test/java/org/springframework/integration/json/SimpleJsonSerializerTests.java
@@ -16,14 +16,16 @@
 
 package org.springframework.integration.json;
 
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.endsWith;
-import static org.hamcrest.CoreMatchers.startsWith;
-import static org.hamcrest.Matchers.arrayWithSize;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 
+import java.util.Map;
+
 import org.junit.Test;
 
+import org.springframework.integration.support.json.JsonObjectMapperProvider;
+
 /**
  * @author Gary Russell
  * @since 5.0
@@ -32,17 +34,16 @@
 public class SimpleJsonSerializerTests {
 
 	@Test
-	public void test() {
+	public void test() throws Exception {
 		Foo foo = new Foo();
-		String json = SimpleJsonSerializer.toJson(foo, "foo", "bar", "dub", "bool");
-		assertThat(json, startsWith("{"));
-		assertThat(json, containsString("\"bool\":true"));
-		assertThat(json, containsString("\"bar\":42"));
-		assertThat(json, containsString("\"foo\":\"bar\""));
-		assertThat(json, containsString("\"dub\":1.6"));
-		assertThat(json, endsWith("}"));
-		String[] split = json.split(",");
-		assertThat(split, arrayWithSize(4));
+		String json = SimpleJsonSerializer.toJson(foo, "fileInfo");
+		@SuppressWarnings("unchecked")
+		Map<String, Object> fromJson = JsonObjectMapperProvider.newInstance().fromJson(json, Map.class);
+		assertThat(fromJson.get("bool"), equalTo(Boolean.TRUE));
+		assertThat(fromJson.get("bar"), equalTo(42));
+		assertThat(fromJson.get("foo"), equalTo("bar"));
+		assertThat(fromJson.get("dub"), equalTo(1.6));
+		assertNull(fromJson.get("fileInfo"));
 	}
 
 	public static class Foo {
@@ -71,6 +72,10 @@ public boolean isBool() {
 			return this.bool;
 		}
 
+		public String fileInfo() {
+			return "foo";
+		}
+
 	}
 
 }
diff --git a/...tion-file/src/main/java/org/springframework/integration/file/remote/AbstractFileInfo.java b/...tion-file/src/main/java/org/springframework/integration/file/remote/AbstractFileInfo.java
@@ -57,8 +57,7 @@ public int compareTo(FileInfo<F> o) {
 	}
 
 	public String toJson() {
-		return SimpleJsonSerializer.toJson(this, "directory", "link", "size", "modified", "filename",
-				"remoteDirectory", "permissions");
+		return SimpleJsonSerializer.toJson(this, "fileInfo");
 	}
 
 }
diff --git a/...org/springframework/integration/file/remote/AbstractRemoteFileStreamingMessageSource.java b/...org/springframework/integration/file/remote/AbstractRemoteFileStreamingMessageSource.java
@@ -58,6 +58,8 @@ public abstract class AbstractRemoteFileStreamingMessageSource<F>
 
 	private final Comparator<AbstractFileInfo<F>> comparator;
 
+	private boolean fileInfoJson = true;
+
 	/**
 	 * the path on the remote server.
 	 */
@@ -112,6 +114,18 @@ public void setFilter(FileListFilter<F> filter) {
 		this.filter = filter;
 	}
 
+	/**
+	 * Set to false to set the {@link FileHeaders#REMOTE_FILE_INFO} header to the raw
+	 * file info object provided by the underlying implementation.
+	 * Default is true meaning that common file information properties are provided
+	 * in that header as JSON.
+	 * @param fileInfoJson false to set the raw object.
+	 * @since 5.0
+	 */
+	public void setFileInfoJson(boolean fileInfoJson) {
+		this.fileInfoJson = fileInfoJson;
+	}
+
 	protected RemoteFileTemplate<F> getRemoteFileTemplate() {
 		return this.remoteFileTemplate;
 	}
@@ -141,7 +155,8 @@ protected Object doReceive() {
 						.setHeader(IntegrationMessageHeaderAccessor.CLOSEABLE_RESOURCE, session)
 						.setHeader(FileHeaders.REMOTE_DIRECTORY, file.getRemoteDirectory())
 						.setHeader(FileHeaders.REMOTE_FILE, file.getFilename())
-						.setHeader(FileHeaders.REMOTE_FILE_INFO, (file.toJson()))
+						.setHeader(FileHeaders.REMOTE_FILE_INFO,
+										this.fileInfoJson ? (file.toJson()) : file.getFileInfo())
 						.build();
 			}
 			catch (IOException e) {

diff --git a/...test/java/org/springframework/integration/ftp/inbound/FtpStreamingMessageSourceTests.java b/...test/java/org/springframework/integration/ftp/inbound/FtpStreamingMessageSourceTests.java
@@ -17,9 +17,9 @@
 package org.springframework.integration.ftp.inbound;
 
 import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 
 import java.io.InputStream;
@@ -36,16 +36,14 @@
 import org.springframework.integration.channel.QueueChannel;
 import org.springframework.integration.config.EnableIntegration;
 import org.springframework.integration.core.MessageSource;
+import org.springframework.integration.endpoint.SourcePollingChannelAdapter;
 import org.springframework.integration.file.FileHeaders;
 import org.springframework.integration.file.remote.session.SessionFactory;
 import org.springframework.integration.ftp.FtpTestSupport;
-import org.springframework.integration.ftp.filters.FtpPersistentAcceptOnceFileListFilter;
 import org.springframework.integration.ftp.session.FtpRemoteFileTemplate;
-import org.springframework.integration.metadata.SimpleMetadataStore;
 import org.springframework.integration.scheduling.PollerMetadata;
 import org.springframework.integration.transformer.StreamTransformer;
 import org.springframework.messaging.Message;
-import org.springframework.messaging.PollableChannel;
 import org.springframework.scheduling.support.PeriodicTrigger;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -61,7 +59,13 @@
 public class FtpStreamingMessageSourceTests extends FtpTestSupport {
 
 	@Autowired
-	public PollableChannel data;
+	private QueueChannel data;
+
+	@Autowired
+	private FtpStreamingMessageSource source;
+
+	@Autowired
+	private SourcePollingChannelAdapter adapter;
 
 	@SuppressWarnings("unchecked")
 	@Test
@@ -88,7 +92,15 @@ public void testAllContents() {
 		assertThat(fileInfo, containsString("filename\":\"ftpSource2.txt"));
 		assertThat(fileInfo, containsString("modified\":"));
 		assertThat(fileInfo, containsString("link\":false"));
-		assertNull(this.data.receive(10));
+
+		this.adapter.stop();
+		this.source.setFileInfoJson(false);
+		this.data.purge(null);
+		this.adapter.start();
+		received = (Message<byte[]>) this.data.receive(10000);
+		assertNotNull(received);
+		assertThat(received.getHeaders().get(FileHeaders.REMOTE_FILE_INFO), instanceOf(FTPFile.class));
+		this.adapter.stop();
 	}
 
 	@Configuration
@@ -113,7 +125,6 @@ public PollerMetadata defaultPoller() {
 		public MessageSource<InputStream> ftpMessageSource() {
 			FtpStreamingMessageSource messageSource = new FtpStreamingMessageSource(template(), null);
 			messageSource.setRemoteDirectory("ftpSource/");
-			messageSource.setFilter(new FtpPersistentAcceptOnceFileListFilter(new SimpleMetadataStore(), "streaming"));
 			return messageSource;
 		}
 

diff --git a/...st/java/org/springframework/integration/sftp/inbound/SftpStreamingMessageSourceTests.java b/...st/java/org/springframework/integration/sftp/inbound/SftpStreamingMessageSourceTests.java
@@ -17,9 +17,9 @@
 package org.springframework.integration.sftp.inbound;
 
 import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 
 import java.io.InputStream;
@@ -35,16 +35,14 @@
 import org.springframework.integration.channel.QueueChannel;
 import org.springframework.integration.config.EnableIntegration;
 import org.springframework.integration.core.MessageSource;
+import org.springframework.integration.endpoint.SourcePollingChannelAdapter;
 import org.springframework.integration.file.FileHeaders;
 import org.springframework.integration.file.remote.session.SessionFactory;
-import org.springframework.integration.metadata.SimpleMetadataStore;
 import org.springframework.integration.scheduling.PollerMetadata;
 import org.springframework.integration.sftp.SftpTestSupport;
-import org.springframework.integration.sftp.filters.SftpPersistentAcceptOnceFileListFilter;
 import org.springframework.integration.sftp.session.SftpRemoteFileTemplate;
 import org.springframework.integration.transformer.StreamTransformer;
 import org.springframework.messaging.Message;
-import org.springframework.messaging.PollableChannel;
 import org.springframework.scheduling.support.PeriodicTrigger;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -62,7 +60,13 @@
 public class SftpStreamingMessageSourceTests extends SftpTestSupport {
 
 	@Autowired
-	public PollableChannel data;
+	private QueueChannel data;
+
+	@Autowired
+	private SftpStreamingMessageSource source;
+
+	@Autowired
+	private SourcePollingChannelAdapter adapter;
 
 	@SuppressWarnings("unchecked")
 	@Test
@@ -89,7 +93,15 @@ public void testAllContents() {
 		assertThat(fileInfo, containsString("modified\":"));
 		assertThat(fileInfo, containsString("link\":false"));
 		assertThat(new String(received.getPayload()), equalTo("source2"));
-		assertNull(this.data.receive(10));
+
+		this.adapter.stop();
+		this.source.setFileInfoJson(false);
+		this.data.purge(null);
+		this.adapter.start();
+		received = (Message<byte[]>) this.data.receive(10000);
+		assertNotNull(received);
+		assertThat(received.getHeaders().get(FileHeaders.REMOTE_FILE_INFO), instanceOf(LsEntry.class));
+		this.adapter.stop();
 	}
 
 	@Configuration
@@ -114,7 +126,6 @@ public PollerMetadata defaultPoller() {
 		public MessageSource<InputStream> sftpMessageSource() {
 			SftpStreamingMessageSource messageSource = new SftpStreamingMessageSource(template(), null);
 			messageSource.setRemoteDirectory("sftpSource/");
-			messageSource.setFilter(new SftpPersistentAcceptOnceFileListFilter(new SimpleMetadataStore(), "streaming"));
 			return messageSource;
 		}
 

diff --git a/src/reference/asciidoc/ftp.adoc b/src/reference/asciidoc/ftp.adoc
@@ -537,7 +537,9 @@ The java configuration below shows one technique to remove the remote file after
 Use the `max-fetch-size` attribute to limit the number of files fetched on each poll when a fetch is necessary; set to 1 and use a persistent filter when running in a clustered environment.
 
 The adapter puts the remote directory and file name in headers `FileHeaders.REMOTE_DIRECTORY` and `FileHeaders.REMOTE_FILE` respectively.
-Starting with _version 5.0_, additional remote file information, in JSON, is provided in the `FileHeaders.REMOTE_FILE_INFO` header.
+Starting with _version 5.0_, additional remote file information, represented in JSON by default, is provided in the `FileHeaders.REMOTE_FILE_INFO` header.
+If you set the `fileInfoJson` property on the `FtpStreamingMessageSource` to `false`, the `FTPFile` object provided by the underlying Apache Net library is used instead.
+This property is not available when using XML configuration but you can set it by injecting the source into one of your configuration classes.
 
 ==== Configuring with Java Configuration
 

diff --git a/src/reference/asciidoc/sftp.adoc b/src/reference/asciidoc/sftp.adoc
@@ -577,6 +577,8 @@ Use the `max-fetch-size` attribute to limit the number of files fetched on each
 
 The adapter puts the remote directory and file name in headers `FileHeaders.REMOTE_DIRECTORY` and `FileHeaders.REMOTE_FILE` respectively.
 Starting with _version 5.0_, additional remote file information, in JSON, is provided in the `FileHeaders.REMOTE_FILE_INFO` header.
+If you set the `fileInfoJson` property on the `SftpStreamingMessageSource` to `false`, the `LsEntry` object provided by the underlying JSch library is used instead.
+This property is not available when using XML configuration but you can set it by injecting the source into one of your configuration classes.
 
 ==== Configuring with Java Configuration
 

diff --git a/src/reference/asciidoc/whats-new.adoc b/src/reference/asciidoc/whats-new.adoc
@@ -85,7 +85,7 @@ See <<ftp-outbound-gateway>> and <<sftp-outbound-gateway>> for more information.
 The FTP and SFTP outbound gateways now support the `REPLACE_IF_MODIFIED` `FileExistsMode` when fetching remote files.
 See <<ftp-outbound-gateway>> and <<sftp-outbound-gateway>> for more information.
 
-The (S)FTP streaming inbound channel adapters now add a JSON representation of the remote file information in a message header.
+The (S)FTP streaming inbound channel adapters now add remote file information in a message header.
 See <<ftp-streaming>> and <<sftp-streaming>> for more information.
 
 ==== Integration Properties