+1 for exposing getCache() on CacheLauncher instances. Death to all singletons!
I'm less certain about the wisdom of exposing a getCache() on LocatorLauncher instances. Seems like it would be better to let clients call getLocator() on LocatorLauncher instances, then they can do the traversal to the cache via getCache() themselves. Would it make sense to expose getLocator() on LocatorLauncher instances (instead of exposing getCache() on those)? If we did implement getCache() for LocatorLauncher instances, it might look something like this: // on LocatorLauncher public Optional<Cache> getCache() { return locator == null ? Optional.empty() : Optional.of(locator.getCache()); } That traversal seems more appropriate for the caller to implement. That way the caller knows why the cache is unavailable (when its unavailable) e.g. because there is no locator vs. because there is a locator but that locator has no cache. On Wed, Oct 31, 2018 at 3:05 PM Dan Smith <dsm...@pivotal.io> wrote: > Yay for APIs that don't require singletons! > > -Dan > > On Wed, Oct 31, 2018 at 2:54 PM Jinmei Liao <jil...@pivotal.io> wrote: > > > +1. sounds like a good addition and since we already have package level > > getters for them anyway. > > > > On Wed, Oct 31, 2018 at 2:48 PM Kirk Lund <kl...@apache.org> wrote: > > > > > LocatorLauncher provides an API which can be used in-process to create > a > > > Locator. There is no public API on that class to get a reference to the > > > Locator or its Cache. > > > > > > Similarly, ServerLauncher provides an API which can be used in-process > to > > > create a Server, but there is no public API in that class to get a > > > reference to its Cache. > > > > > > The User of either Launcher would then have to resort to invoking > > > singletons to get a reference to the Cache. > > > > > > There are existing package-private getter APIs on both Launchers but > > > they're only used by tests in that same package. > > > > > > I propose adding public APIs for getCache to both LocatorLauncher and > > > ServerLauncher as well as adding getLocator to LocatorLauncher. The > > > signatures would look like: > > > > > > /** > > > * Gets a reference to the Cache that was created by this > ServerLauncher. > > > * > > > * @return a reference to the Cache > > > */ > > > public org.apache.geode.cache.Cache getCache(); > > > > > > /** > > > * Gets a reference to the Locator that was created by this > > > LocatorLauncher. > > > * > > > * @return a reference to the Locator > > > */ > > > public org.apache.geode.distributed.Locator getLocator(); > > > > > > Any thoughts? Yay or nay? > > > > > > Thanks, > > > Kirk > > > > > > > > > -- > > Cheers > > > > Jinmei > > >