Improve Data Retention Settings

[gessnerfl]

With #517 parameters to configure the fixedDelay and the initialDelay of the email data retention timer. Although the chosen design fits into the current implementation it is not open for future extensions.

fakesmtp:
  persistence:
    maxNumberEmails: 100
    fixedDelay: 300000
    initialDelay: 60000

In case another timer is added to the persistence model, the configuration values would be ambiguous. Because of this, the design should be changed such as that on the one hand side the configuration parameters are more descriptive and on the other hand allows future extensions.

fakesmtp:
  persistence:
    maxNumberEmails: 100
    emailDataRetentionTimer:
       fixedDelay: 300000
       initialDelay: 60000

With this the parameters and their purpose should be self descriptive and allow adding additional data retention configurations as well as any other timer configuration as needed.

[gottschd]

Hi @gessnerfl,
hello together,

I can fully understand the intention of this proposal and i welcome and support it for the sake of future evolution.

However, please allow me to throw my (very subjective) towel into the ring for the configuration parameter fakesmtp.persistence.maxNumberEmails. IMHO this parameter defines a very important feature of this software and so might be properly heavily used.

With this proposal, the parameter will now become non-functional and existing configurations that changed the default to any custom value (with all supported spring ways) will see a behavioral change.

This is properly the reason that this issue is marked as "breaking".

Is there a chance that the old configuration parameter can be supported before it is finally removed, i.e to give users a chance to adapt?

On the other hand, maybe I'm overthinking the benefit for my request of supporting the old parameter as it might become a maintenance/documentation/clearness burden.

Do you have some convincing words/reasoning for me 😃 ?

Kind regards,
Danny

[gessnerfl]

@gottschd
Actually, I agree with you. Although I think the proposed solution is clean but it is maybe also a bit over designed. I decided to simplify it and keep it backward compatible. (see the changed configuration above).

[gessnerfl]

The changes are incorporated into the pull request

[gottschd]

@gessnerfl
I like your improved proposal.
It is, as you already stated, backward-compatible but also open for future extensions.

Thanks a lot.

[gessnerfl]

The issue is resolved with version 2.4.0