Add support for loading library configuration from file in Bsz\Config…

[yanchun-bsz]

No description provided.

[sebsahli]

Looks good! However, I would consider taking advantage of the \VuFind\Config\Feature\SecretTrait, which already provides the getSecretFromConfig method for exactly our purpose (read secret from file, if available and fallback to config if there is no file). Thus, we can avoid code duplication and rely on code maintained by the community.

[yanchun-bsz]

SecretTrait works nicely in Koha, since it is an object instance. Factory::getLibrariesTable() is static, so getSecretFromConfig() cannot be called directly.

[sebsahli]

Oh yes, you are right. Didn' think that through... In this case your way seems appropriate. However, in general, we want to try to avoid copying code from VuFind. In this case, an option would be to use a dedicated LibrariesTableFactory class instead of the static getLibrariesTable method. BOSS uses a lot of those static factory methods, while VuFind (and Laminas) prefers the factory approarch. So in the long run I also want to switch to factory classes. Then we can rework this part of code again, but for now it is fine :)

[yanchun-bsz]

Thanks! Sounds good. I agree that a dedicated LibrariesTableFactory would be a cleaner solution. We can revisit this when we refactor the factories 🙂

[sebsahli]

Oh yes, you are right. Didn' think that through... In this case your way seems appropriate. However, in general, we want to try to avoid copying code from VuFind. In this case, an option would be to use a dedicated LibrariesTableFactory class instead of the static getLibrariesTable method. BOSS uses a lot of those static factory methods, while VuFind (and Laminas) prefers the factory approarch. So in the long run I also want to switch to factory classes. Then we can rework this part of code again, but for now it is fine :)