stoerr edited a comment on pull request #136: URL: https://github.com/apache/jackrabbit-filevault/pull/136#issuecomment-828974792
I did consider getting rid of lazy loading completely when doing #135 , but I was afraid the original author might have had a good reason to do it. The problem is that the method loadPackages() is a quite expensive method, as it accesses the filesystem and opens all files to collect metadata. If the filesystem is a network filesystem (which might even make sense for distributing packages across instances), then this can even block indefinitely. That's fine in tests (at least within Jackrabbit Vault the FSPackageRegistryIT and your new test seem to be the only places the additional constructors are used), but that might not be OK during OSGI activation. Could that block your bundles from coming up? And the registry might even not be used at all later... You could consider encapsulating stateCache to switch it completely to lazy loading to be consistent (in the original code getInstallState / setInstallState methods also didn't call the loadPackages()), but that does change the behavior of the public constructors somewhat, so it might theoretically break e.g. a test someone did in a project using Vault, though that's admittedly a strange case. Not sure whether it's worth the risk. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
