spring-projects / spring-integration-extensions

The Spring Integration Extensions project provides extension components for Spring Integration
http://www.springintegration.org/
280 stars 266 forks source link

SMB with custom transport configuration #212

Closed ceremo closed 5 years ago

ceremo commented 5 years ago

Hi, We want to change the transport configuration like timeouts, this is defined as a configuration of the CIFSContext, and the SessionFactory with each session creates a new SmbShare that uses always the SingletonContext, it will be great that we can pass to a session factory a context with a custom configuration.

Thxs.

GregBragg commented 5 years ago

There are a lot of jcifs.smb.client.* properties in the jcifs.config.PropertyConfiguration class that can be set via configuration in the dependent jCIFS library. Can you review this class please to see if the defaults are sufficient?

Which specifically besides timeout are you considering? I'm not sure if exposing all the jCIFS properties is really feasible at this point.

ceremo commented 5 years ago

Thxs for you quickly answer @GregBragg, I'm not looking for having all the properties available, what I'd like is to be able to pass a custom BaseContext to the SessionFactory, and if this is defined work with it, and if this is not available uses the SingletonContext.

GregBragg commented 5 years ago

So you are looking to extend jcifs.context.BaseContext or the base class jcifs.context.AbstractCIFSContext to add some further custom implementation? Can you give me an example of why you need to do this?

ceremo commented 5 years ago

We need to have some SMB clients, and each of them with different transport configurations using a BaseContext to keep this custom configuration, to achieve this goal, I was expecting that SmbShare has a constructor like SmbFile (public SmbFile ( URL url, CIFSContext tc )), and SmbSessionFactory has also an additional constructor with the parameter CIFSContext, so when SmbSessionFactory has to create a new session using SmbShare, it checks if this CIFSContext has value, if it has value will pass the context to the SmbShare, if not should use SmbShare with the SingletonContext (default configuration).

It would be great to have this.

GregBragg commented 5 years ago

I'll look into it and create a separate feature branch in my fork... I'll need to run the design past @artembilan for final approval.

In the meantime can you please provide some test classes that implement the CIFSContext for my JUnit tests and the expected results. You can zip them up and attach it here to the issue.

artembilan commented 5 years ago

Yes, I found this request as valid. Since SmbShare extends SmbFile and the last one has a ctor like:

public SmbFile ( String url, CIFSContext tc ) throws MalformedURLException {

We definitely have to expose such an API on our level as well.

In the end this is already end-user responsibility to use our API a proper way and if an underlying library provides some tricks, there is no reason to hide them.

GregBragg commented 5 years ago

I've made the changes in a feature branch in my fork: https://github.com/spring-projects/spring-integration-extensions/compare/master...GregBragg:SMB_with_custom_transport_configuration_%23212

@ceremo I'll still need the test classes before I can create a PR for this.

ceremo commented 5 years ago

I'm creating an ordinary BaseContext with the configuration already customized, for the configuration I created a class extending of BaseConfiguration to be able to use @ConfigurationProperties and load the config using properties, this was because some getters and setters don't match with the class variables, and that's all. To inject this context to the SmbShare I had to use reflection, but with your solution I'll be able to remove this.

For the tests of this change, I won't complicate more than verifying if when the SessionFactory is creating a new session, is using the BaseContext passed by constructor.