[#252] Support for 'addresshierarchy' domain. #254
Conversation
|
|
||
| try { | ||
| AddressConfigurationLoader.loadAddressConfiguration(Paths.get(iniz.getConfigDirPath()), Paths.get(iniz.getChecksumsDirPath()), getDomain().toString()); | ||
| Context.getService(AddressHierarchyService.class).initI18nCache(); |
There was a problem hiding this comment.
@mogoodrich this is always gona initialize the i18n cache at startup from here. initializeFullAddressCache() is implicitly called by AddressConfigurationLoader.loadAddressConfiguration(...)
There was a problem hiding this comment.
I've also tweaked this to ensure the initializeFullAddressCache is always called not only when checksums are changed.
There was a problem hiding this comment.
It would seem like Initializer should stick to loading in AH entries, and not try to take responsibllity over the full address cache or the i18n cache. I'd be more inclined to make changes to the AddressConfigurationLoader.loadAddressConfiguration method to ensure these caches are initialized whenever it is called. Basically at the bottom of AddressConfigurationLoader.loadAddressConfiguration:
1 If the AH entries are changed and reloaded, it should reload these caches
2.If the AH entries are not changed and reloaded, but the caches have not been initialized yet, reload these caches
@mogoodrich does that make sense?
There was a problem hiding this comment.
Yes, @mseaton that is my preference as well, that Iniz sticks to loading metadata, and not being responsible for setting up in-memory caches.
I believe the original loadAddressConfiguration method already did #1: reloads the cache if the AH entries are reloaded. #2 could be added as well, but is not really necessary if the if we keep the method that refreshes the cache in the contextRefreshed method of the Address Hierarchy activator.
@Ruhanga @mks-d I assume the intent of Iniz is primarily to do metadata loading, not to do overall module start-up tasks? For instance, if for some reason Iniz was uninstalled from an implementation, I'd still expect a module to start up properly, as long as it's metadata was already loaded?
There was a problem hiding this comment.
I assume the intent of Iniz is primarily to do metadata loading, not to do overall module start-up tasks?
Yes, I think that's explicitly the purpose here.
There was a problem hiding this comment.
For instance, if for some reason Iniz was uninstalled from an implementation, I'd still expect a module to start up properly, as long as it's metadata was already loaded?
@mogoodrich, @ibacher, this would cause some malfunctioning already(as is the case) since Iniz does load the i18n message properties necessary for proper cache initialization from the now proxy addresshierarchy domain (in the context of Iniz).
|
|
||
| try { | ||
| AddressConfigurationLoader.loadAddressConfiguration(Paths.get(iniz.getConfigDirPath()), Paths.get(iniz.getChecksumsDirPath()), getDomain().toString()); | ||
| Context.getService(AddressHierarchyService.class).initI18nCache(); |
There was a problem hiding this comment.
It would seem like Initializer should stick to loading in AH entries, and not try to take responsibllity over the full address cache or the i18n cache. I'd be more inclined to make changes to the AddressConfigurationLoader.loadAddressConfiguration method to ensure these caches are initialized whenever it is called. Basically at the bottom of AddressConfigurationLoader.loadAddressConfiguration:
1 If the AH entries are changed and reloaded, it should reload these caches
2.If the AH entries are not changed and reloaded, but the caches have not been initialized yet, reload these caches
@mogoodrich does that make sense?
api/src/test/java/org/openmrs/module/initializer/api/AddressHierarchyLoaderIntegrationTest.java
Show resolved
Hide resolved
mks-d
changed the title
mks-d
changed the title
#252