Skip to content

Add file-split-ftp Sample #177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Add file-split-ftp Sample #177

wants to merge 3 commits into from

Conversation

garyrussell
Copy link
Contributor

@artembilan
Copy link
Member

Well, welcome back and your test have been failed 😄

org.springframework.integration.samples.filesplit.ApplicationTests > testFailure FAILED

    java.lang.IllegalStateException

        Caused by: org.springframework.beans.factory.BeanCreationException

            Caused by: org.springframework.beans.factory.BeanCreationException

                Caused by: org.springframework.beans.factory.NoSuchBeanDefinitionException

org.springframework.integration.samples.filesplit.ApplicationTests > testSuccess FAILED

    java.lang.IllegalStateException

        Caused by: org.springframework.beans.factory.BeanCreationException

            Caused by: org.springframework.beans.factory.BeanCreationException

                Caused by: org.springframework.beans.factory.NoSuchBeanDefinitionException

@garyrussell
Copy link
Contributor Author

Strange, runs fine with gradle for me locally; re-pushed with top level README update. Let's see...

artembilan
artembilan requested changes Sep 20, 2016
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's all. Just several nit-picking to consider or just discuss. But I won't mind if you address them in subsequent commit.

applications/file-split-ftp/pom.xml
<modelVersion>4.0.0</modelVersion>

<groupId>com.example</groupId>
<artifactId>vg</artifactId>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need POM in this project any way, but that should be correct one based on the group = 'org.springframework.integration.samples'

build.gradle
}

task run(type: JavaExec) {
main 'org.springframework.integration.samples.barrier.Application'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong class name. In both places.

...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java
import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;

@SpringBootApplication
@EnableConfigurationProperties(MailProperties.class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems for me we don't need this.
It is a part of MailSenderAutoConfiguration.

I won't mind if I miss something. Then any explanations are welcome.
Thanks

...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java
}

private PayloadTypeRouter ptr() {
PayloadTypeRouter payloadTypeRouter = new PayloadTypeRouter();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit-picking, of course, but can't we rely on the Java DSL 1.2 here already:

.<Object, Class<?>>route(Object::getClass, m -> m
    .channelMapping(FileSplitter.FileMarker.class, "markers.input")
    .channelMapping(String.class, "lines.input"))

?

...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java
@Bean
public IntegrationFlow markers() {
return f -> f.<FileSplitter.FileMarker>filter(m -> m.getMark().equals(FileSplitter.FileMarker.Mark.END),
e -> e.id("markerFilter").discardChannel("nullChannel")) // INT-4109
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://jira.spring.io/browse/INT-4109 has been fixed and Samples is on the SI-4.3.2 already.

...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java
.handleWithAdapter(a -> a.ftp(ftp3()).remoteDirectory("foo"), e -> e.id("ftp009")))

// send an email
.subscribe(sf -> sf.<FileSplitter.FileMarker, String>transform(p -> p.getFilePath())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My IDEA says that can be replaced with method reference:

.subscribe(sf -> sf.transform(FileSplitter.FileMarker::getFilePath)

...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java
StringBuilder builder = new StringBuilder();
builder.append(p.getFailedMessage().getPayload().toString()).append("\n")
.append(getStackTraceAsString(p));
return builder.toString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really don't need StringBuilder.
The compiler converts simple string concatenation to that one.

...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java
public IntegrationFlow fromFile() {
return IntegrationFlows.from(Files.inboundAdapter(new File("/tmp/in"))
.patternFilter("*.txt"), e -> e.poller(Pollers.fixedDelay(5000)))
.enrichHeaders(h -> h.header(MessageHeaders.ERROR_CHANNEL, "tfrErrors.input"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://jira.spring.io/browse/INT-4113.
I mean add comment here.
Although I think after getComponentsToRegister() for the PollerSpec we can provide some out-of-the-box workaround just with errorChannel(MessageChannelString)
But right: that is a different story...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to pollute the code here with a TODO: will add a comment to the JIRA.

...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java
.enrichHeaders(Mail.headers()
.subject("File successfully split and transferred")
.from("foo@bar")
.toFunction(m -> new String[] {"bar@baz"}))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! These are up to @ConfigurationProperties. Our own custom one.
I mean for the real mail server tests.
But I see we have so many of options to expose...

Maybe we can live without that for now...

artembilan
artembilan reviewed Sep 20, 2016
View reviewed changes
...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java
@Bean
public IntegrationFlow lines() {
return f -> f.enrichHeaders(h -> h.headerExpression(
FileHeaders.FILENAME, "payload.substring(1, 4) + '.txt'", true))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT, if we change this .enrichHeaders() just to the .fileNameExpression("payload.substring(1, 4) + '.txt'")?

Although it doesn't work with your test somehow... But I see in debug the proper expression and expected file name result.

artembilan
artembilan reviewed Sep 21, 2016
View reviewed changes
...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java

// first trigger file flushes
.subscribe(sf -> sf.transform("'/tmp/out/.*\\.txt'", e -> e.id("toTriggerPattern"))
.handle("fileOut.handler", "trigger", e -> e.id("flusher")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the problem with bean definitions order on Linux we should move Files.outboundAdapter() to the separate @Bean. At least until the fix for https://github.com/spring-projects/spring-integration-java-dsl/issues/119

artembilan
artembilan reviewed Sep 21, 2016
View reviewed changes
...e-split-ftp/src/main/java/org/springframework/integration/samples/filesplit/Application.java
@Bean
public IntegrationFlow lines() {
return f -> f.enrichHeaders(h -> h.headerExpression(
FileHeaders.FILENAME, "payload.substring(1, 4) + '.txt'", true))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gary, any ideas why it doesn't work with just .fileNameExpression() and I'll merge with the fix for handler bean access.
Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will look.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It worked fine for me with the file name expression. I've built on linux too. Let's see what our friend Travis thinks.

@artembilan
Copy link
Member

Merged as 2c1a070.

See my polishing to the tests according Windows compatibility.

@artembilan artembilan closed this Sep 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artembilan artembilan artembilan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@garyrussell @artembilan