• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Is this the Use Case for Factory Pattern?

 
Greenhorn
Posts: 18
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi! I am working on a app which uses SFTP protocol to upload a file to an SFTP server.

Now the project manager has tasked me to include MinIO to upload files using a configuration switch.

1) Is this the situation where I can use Factory Pattern to apply it?  
2) How can I minimally change the SFTP protocol so that I do not break the code?

As I am a newbie in Design Patterns, please advise. Thanks.
 
Saloon Keeper
Posts: 15510
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
We don't have enough information to make that determination for you. Software design patterns are applicable in a very wide range of scenarios.

Instead of asking us if the factory pattern is appropriate, can you explain to us how you plan to use it, and why you think it might help in this case?
 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

A Tham wrote:2) How can I minimally change the SFTP protocol so that I do not break the code?


Sounds like a nonstarter to me.

As I understand it, MinIO is cloud-based object storage. SFTP is a transfer protocol.  These are two entirely different concerns so the only "pattern" I see in "modifying SFTP protocol to include MinIO" is the antipattern of mixing two unrelated concerns.

But to Stephan's point, you should tell us the details so we have more context as to what exactly you plan to do.
 
A Tham
Greenhorn
Posts: 18
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi guys! Thanks for your replies.

I have an app to upload some files into a file server. It is currently using SFTP protocol to upload the files. Now the manager wants me to change the app to switch between SFTP file upload and MinIO cloud-based object storage based on a config key.

Problem is: The service to process the files is tightly coupled to the SFTP protocol. For example, in the ProofProcessorService class, I have:



There are other codes which does other processing before uploading using SFTP.

Now, I need to change lines 9, 11-16 to have the option to use MinIO to upload the files. I am thinking of using Factory Pattern to create the object to upload the files. Code looks something like this:



After that, I will change storeProofs() to use the fileProtocolFactory,  generate the appropriate FileTransferService , call uploadFiles method to upload the files.

I will need to change the current FileTransferService to become SFTPFileTransferService.
I will also need to create a new MinioFileTransferService to implement FileTransferService.

So both SFTPFileTransferService and MinioFileTransferService will implement FileTransferService.

Is my design correct? Please enlighten. Thanks.
 
Stephan van Hulst
Saloon Keeper
Posts: 15510
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Yes, the factory pattern could be a good solution for your problem. Personally I prefer the abstract factory pattern though.


Inject the factory into your proof storing service and call it from the storeProofs() method:

An implementation for SFTP:

And here's how you can annotate the factory implementation so that Spring injects it conditionally:



Note the following improvements I made to your code:

  • Don't use numerical types for IDs. There's never a reason to treat an ID as numerical. You're not going to perform math on it. In fact, you're even converting one of your IDs to String.
  • If you MUST use a numerical type, use a primitive. Why is your method accepting Long instead of long?
  • Limit the use of String. Use URL for urls. Use LocalDate for dates. Use strongly typed Customer and Organization classes.
  • If your baseDir refers to a directory on the local filesystem, as opposed to a directory on the system you're SFTPing to, use Path instead of String.
  • Calling close() in a finally block is very old-school and easy to get wrong. Look at how much cleaner try-with-resources looks in my storeProofs() method.
  • Don't throw or catch Exception. Use more specific exception types, such as FileTransferException (which I made up), or if you need more flexibility, IOException.
  • Autowire into private final fields through a constructor. This allows you to validate them and make the fields final.
  •  
    A Tham
    Greenhorn
    Posts: 18
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Hi Stephan, Thanks for your reply. Great, detailed solution.

    Just one thing: you didn't specify an @Service for SftpFileTransferService. I assume it is because this class is created using the new keyword in SftpFileTransferServiceFactory.

    But I would need some variables to be initialized by autowiring. They are null even though I have put @Autowired.
    Eg:

    @Autowired
    private OrderImageRepository orderImageRepository;

    How should I go about this? Thanks.
     
    Junilu Lacar
    Sheriff
    Posts: 17644
    300
    Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    A Tham wrote:Is my design correct? Please enlighten. Thanks.


    It seems that you're slowly getting there by introducing a factory of some sort. Just looking at the code, however, I think it's still too tightly coupled and that several dependencies between concerns could be loosened up.

    I'm partial to Alistair Cockburn's hexagonal architecture style so if I were given this code to maintain, I'd probably refactor towards that.

    In a hexagonal architecture, any infrastructure concerns like saving or transferring a file is kept decoupled from core business logic like processing a proof, whatever that is in your application. So the ProofProcessingService would be a core-based object/class whereas the SftpTransfer and MinIOTransfer classes would be implementations of an infrastructure-related interface, much like what Stephan is headed toward.

    There would be a high reliance on dependency injection in a design that follows the hexagonal architecture. So the ProofProcessingService would be simply be injected with an implementation of the FileTransferService.

    Something that stands out as peculiar in your design is the fact that the storeProofs() method takes a FileData parameter and returns the same object. The semantics of that design seems strange to me and makes me question why that's being done.

    Ideally, I might expect to see this kind of code:

    This isn't as detailed or specific as what Stephan provided but it shows the separation of concerns that I would strive for. The issue I have with your current design is that saveProofs() knows too much about the details of locating a file and transferring a file. I feel that some of that intimate knowledge should be moved to a more appropriate class whose responsibility it is to know that kind of detail.
     
    Stephan van Hulst
    Saloon Keeper
    Posts: 15510
    363
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    A Tham wrote:you didn't specify an @Service for SftpFileTransferService. I assume it is because this class is created using the new keyword in SftpFileTransferServiceFactory.


    Correct.

    But I would need some variables to be initialized by autowiring. They are null even though I have put @Autowired.


    Do you mean that inside my example of either SftpFileTransferService or MinIoFileTransferService (which are instantiated through the new keyword and not through Spring) you need an additional field of type OrderImageRepository?

    If this is the case, first I question the need of a repository in a service whose sole task should be to upload or download files.

    However, if you REALLY want to do this, all you need to do is add a parameter of that type to the constructor and pass an instance of the repository in when you use the new keyword inside the factory class. You can then autowire the instance you need into the factory class:

    But please explain to us why you would need this.
     
    A Tham
    Greenhorn
    Posts: 18
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Thank you Stephan and Junilu.
     
    Don't get me started about those stupid light bulbs.
    reply
      Bookmark Topic Watch Topic
    • New Topic