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

ExchangeRateProvider implementations shouldn't load data within constructor #367

Open
2kewl4u opened this issue Jul 29, 2021 · 0 comments
Open
Labels
Milestone

Comments

@2kewl4u
Copy link

2kewl4u commented Jul 29, 2021

During startup of the application, the resources for exchange rates might be loaded until 3 times.

  1. The constructor of ECBAbstractRateProvider calls LoaderService.loadDataAsync(..) and IMFHistoricRateProvider, IMFRateProvider do call LoaderService.loadData(..).
  2. Before that, the call to Bootstrap.getService(LoaderService.class) will create the DefaultLoaderService that on initialize() will cause the LoaderConfigurator to register the LoadDataInformation. Depending on the configuration this will trigger the DefaultLoaderService to read from remote if the property 'startRemote' is set to true.
  3. Furthermore DefaultLoaderService.registerData(..) will then trigger the loading again depending on the update policy.

All these methods will somehow end up calling LoadableResource.load(), which is configured to cache the resource data, however the class itself is not thread-safe, thus depending on the timing when these methods are called, the resource is still loaded multiple times on startup.

Since the ExchangeRateProvider implementations want to register themselves to the LoaderService and the creation of this service already triggers the resources to be loaded, I recommend to remove the loading of the resources from ExchangeRateProvider constructors. I would consider it anyway as a flaw to directly load data within the constructor of these providers. Since all abstract classes for these providers work anyway with a 'loadLock', it would not cause any harm if the loading process is removed from the constructors and left to the LoaderService, where it can be configured and also deactivated if necessary.

@keilw keilw added the deferred label Sep 8, 2021
@keilw keilw added this to the 1.5 milestone Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants