Thanks for pointing out that DistributionManager is internal -- I forgot about that. I'm primarily concerned with internal Functions, such as those for GFSH commands, so maybe an internal version of FunctionContext which exposes more would be good for those.
On Wed, May 24, 2017 at 11:39 AM, Darrel Schneider <dschnei...@pivotal.io> wrote: > FunctionContext is an external interface so it can not expose internal > interfaces like DistributionManager. > But it could expose Cache. DistributedSystem is external so you could have > it exposed from FunctionContext but it is already exposed from Cache. > SecurityService is also internal. > Are you thinking that for internal Functions you would cast FunctionContext > to an internal that would then expose these internal classes? > > > > On Thu, May 18, 2017 at 5:13 PM, Kirk Lund <kl...@apache.org> wrote: > > > I've been digging through our code with close attention to the > singletons. > > I believe the majority of singletons in Geode exist for two main reasons: > > > > 1) Insufficient context or lack of service lookup for Function, > > DistributionMessage and (Client)Command implementations. > > > > 2) Poor OO design. This is where you see code in one class invoking > > concrete methods on another class outside of its concerns. Many of these > > need to be teased apart and replaced with some sort of Observer that > > isolates the reaction from the source of the originating event. > > > > Right now my focus is on #1 because that's the area that's currently an > > obstacle for me. > > > > Function, DistributionMessage and (Client)Command classes need to have > more > > context provided to them (Cache, Security, etc) or they need a better > > mechanism to look up these services. Currently these classes reach out to > > singletons in order to "get" what they need. > > > > *A) Function* > > > > The main entry-point which injects services into the Function is: > > > > public void execute(FunctionContext<T> context); > > > > The FunctionContext needs to provide the service(s) that any Function > might > > require. This could include Cache, DistributionManager and maybe > > SecurityService (anything else?). > > > > *B) (Peer-to-peer) DistributionMessage* > > > > The main entry-point which injects services into the DistributionMessage > > is: > > > > protected abstract void process(DistributionManager dm); > > > > We could provide multiple arguments or a single new DistributionContext > > which then provides DistributionManager and Cache (anything else?). > > > > *C) (Client) Command* > > > > The main entry-point which injects services into the Command is: > > > > public void execute(Message msg, ServerConnection servConn); > > > > ServerConnection is huge but it does already expose Cache. BaseCommand is > > the only Command that implements "execute" and it defines a new > entry-point > > for injection: > > > > abstract public void cmdExecute(Message msg, ServerConnection servConn, > > long start) throws IOException, ClassNotFoundException, > > InterruptedException; > > > > We might want to clean that up and define a new CommandContext which > > provides the Cache or anything else that the Command may need. > > > > Thoughts or additional ideas? > > >
