Skip to content
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.

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

SFTP Outbound Gateway: UnsatisfiedDependencyException when no expression is specified #3395

Closed
mauromol opened this issue Oct 1, 2020 · 5 comments · Fixed by #3397
Closed
Assignees
Milestone

Comments

@mauromol
Copy link
Contributor

mauromol commented Oct 1, 2020

In what version(s) of Spring Integration are you seeing this issue?

5.3.2.RELEASE

Describe the bug

When you define a SFTP outbound gateway using the XML namespace, if you don't specify the expression attribute, instantiation of the gateway fails with an obscure message.

To Reproduce

Consider this:

<int-sftp:outbound-gateway
  id="myGateway"
  request-channel="inputChannel"
  reply-channel="outputChannel"
  session-factory="mySessionFactory"
  remote-directory="${sftp.base-path}"
  command="put" />

<bean id="mySessionFactory"
  class="org.springframework.integration.file.remote.session.CachingSessionFactory">
    <constructor-arg>
      <bean class="org.springframework.integration.sftp.session.DefaultSftpSessionFactory">
        <property name="host" value="${sftp.host}" />
        <property name="port" value="${sftp.port}" />
        <property name="user" value="${sftp.username}" />
        <property name="password" value="${sftp.password}" />
        <property name="knownHostsResource" value="classpath:/config/integration/sftp_host" />
        <property name="timeout" value="${sftp.timeout}" />
      </bean>
    </constructor-arg>
    <constructor-arg value="3" />
    <property name="sessionWaitTimeout" value="120000" />
    <property name="testSession" value="true" />
</bean>

Expected behavior

This is expected to work. As documentation for put command says, "the payload of the message can be a java.io.File, a byte[], or a String". So, it's natural for me that the "default expression" is the message payload, unless I want to submit something else. This is quite standard in Spring Integration.

(quick hint: what about java.nio.file.Path support?)

Actual behavior

This fails on application context refresh with:

org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'org.springframework.integration.sftp.gateway.SftpOutboundGateway#0' defined in class path resource [config/integration/applicationContext-integration-sftp.xml]: Unsatisfied dependency expressed through constructor parameter 0: Could not convert argument value of type [org.springframework.integration.sftp.session.SftpRemoteFileTemplate] to required type [org.springframework.integration.file.remote.session.SessionFactory]: Failed to convert value of type 'org.springframework.integration.sftp.session.SftpRemoteFileTemplate' to required type 'org.springframework.integration.file.remote.session.SessionFactory'; nested exception is java.lang.IllegalStateException: Cannot convert value of type 'org.springframework.integration.sftp.session.SftpRemoteFileTemplate' to required type 'org.springframework.integration.file.remote.session.SessionFactory': no matching editors or conversion strategy found
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:765) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:227) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1356) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1203) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:556) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:516) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:324) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:226) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:322) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:202) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:897) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:879) ~[spring-context-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:551) ~[spring-context-5.2.8.RELEASE.jar:5.2.8.RELEASE]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:143) ~[spring-boot-2.3.3.RELEASE.jar:2.3.3.RELEASE]

Workaround

Add expression="payload" to the outbound gateway definition.

This was also submitted to Stack Overflow at: https://stackoverflow.com/questions/64160194/unsatisfieddependencyexception-when-setting-up-a-spring-integration-sftp-outboun

@mauromol mauromol added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Oct 1, 2020
@mauromol
Copy link
Contributor Author

mauromol commented Oct 1, 2020

Just an additional note: if you really really want the user to be forced to specify the expression attribute even when the message payload is the desired gateway input (but why?), I think the XML schema documentation should be improved to document this (right now it just says SpEL expression representing the path in the command (e.g. ls path to list the files in directory path).) and perhaps the parser may fail with a more meaningful error message if that attribute is missing.

@garyrussell
Copy link
Contributor

This is valid; the issue is because the gateway can be used for many functions and and not all require an expression.

This is hard to do in a parser because the command (e.g. get) can be a property placeholder that can be resolved later, so we can't make the decision in the parser. (It's probably unlikely to use a PPH here, but it's allowed).

We either need to create a factory bean so we can perform the check after the place holders have been resolved, or we might be able to set a default in the .xsd, but we'd have to check if there are any unexpected side effects of doing that (with the other commands).

@garyrussell garyrussell removed the status: waiting-for-triage The issue need to be evaluated and its future decided label Oct 1, 2020
@garyrussell garyrussell added this to the Backlog milestone Oct 1, 2020
@garyrussell
Copy link
Contributor

Please open a separate new feature issue to request Path support.

@garyrussell
Copy link
Contributor

This is the assertion in the gateway...

if (expression == null) {
	Assert.state(Command.LS.equals(this.command)
					|| Command.NLST.equals(this.command)
					|| Command.PUT.equals(this.command)
					|| Command.MPUT.equals(this.command),
			"Only LS, NLST, PUT and MPUT commands can rely on the working directory.\n" +
					"All other commands must be supplied with the filename expression");
	this.fileNameProcessor = null;
}

PUT and MPUT ignore the expression altogether (they always use the payload).

Adding a hard-coded payload in the .xsd would break LS and NLST because they would attempt to list the contents of the payload evaluated to a remote directory instead of the current working directory after logging on to the remote server. So, unfortunately, that's not an option (without adding some hacks).

@garyrussell
Copy link
Contributor

Correction - since this error is at the parser level, an expression is required, even for those commands that don't need it; therefore there is a second bug - the parser doesn't support LS with no expression (list all files from the CWD).

@garyrussell garyrussell self-assigned this Oct 2, 2020
@garyrussell garyrussell modified the milestones: Backlog, 5.4 RC1 Oct 2, 2020
garyrussell added a commit to garyrussell/spring-integration that referenced this issue Oct 2, 2020

Verified

This commit was signed with the committer’s verified signature.
shuding Shu Ding
Resolves spring-projects#3395

Outbound remote file gateway parser requires `expression` even though some
commands don't need or use it.

To avoid adding a factory bean, set a default value in the schema and interpret
that default as needed, depending on the command.
artembilan pushed a commit that referenced this issue Oct 2, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Resolves #3395

Outbound remote file gateway parser requires `expression` even though some
commands don't need or use it.

* Propagate the empty string for `expression` attribute
* Fix default (`payload`) expression logic in the `AbstractRemoteFileOutboundGateway`

**Cherry-pick to `5.3.x`**
artembilan pushed a commit that referenced this issue Oct 2, 2020

Verified

This commit was signed with the committer’s verified signature.
shuding Shu Ding
Resolves #3395

Outbound remote file gateway parser requires `expression` even though some
commands don't need or use it.

* Propagate the empty string for `expression` attribute
* Fix default (`payload`) expression logic in the `AbstractRemoteFileOutboundGateway`

**Cherry-pick to `5.3.x`**
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants