Hi, Remove REPOSITORY_INSTANCES: I think that's generally a good idea.
However it would be strange if a repository factory can only create one repository. But instead of an ownRepositories we could add a 'back reference' from the repository to the factory that created it. Regards, Thomas On Wed, Oct 28, 2009 at 1:53 PM, Jukka Zitting <[email protected]> wrote: > Hi, > > As discussed in JCR-2360, I was looking at pushing the repository URI > support down to implementation-specific repository factories. > > While doing so, I ran into the REPOSITORY_INSTANCES map and > ownRepositories sets in o.a.j.core.RepositoryFactoryImpl. Do we need > them, or would it be better to not keep track of the repository > instances across the factory instances. > > The current behaviour is designed to make the following code pass: > > Repository a = JcrUtils.getRepository("file://path/to/repository"); > Repository b = JcrUtils.getRepository("file://path/to/repository"); > assertTrue(a == b); > > Do we need this, or would it be better if we just instantiated a new > repository for each getRepository call and let the latest repository > access fail because the repository has already been started. I don't > think too many clients would be adversely affected by this change, as > such use of an embedded repository is typically only needed in cases > where you don't have multiple clients accessing the repository. > > The benefit of not keeping the static REPOSITORY_INSTANCES map is that > there'd be no unexpected references to the repository instances and > thus they'd get normally garbage collected when no longer used. This > isn't too much of a problem now given the way TransientRepository > works, but it might prove troublesome later on. > > Also, instead of the ownRepositories set used to track repository > instances created by a factory, we could just keep a single repository > reference in the factory instance. Any subsequent getRepository() > calls could simply throw an InvalidStateException, as the standard > access mechanisms documented for RepositoryFactory all call for a new > factory instance to be created for each getRepository() call. > > These changes would simplify the RepositoryFactoryImpl implementation > and prevent potential memory issues in the future. > > BR, > > Jukka Zitting >
