This proposal contains a number of improvements to the function service to improve usability issues.

Improvements

Minor changes

There are some minor improvements that already have JIRAs. We should just apply the changes described in these jiras.

...

RegionFunctionContext should provide the local data set

If you want to operate on the local data set for a function, you have to do this

...

See

Functions executed on a region should implement a different interface

Any function that is executed on a region has to do a cast:

...

See

The methods isHA, hasResult, optimizeForWrite should move from the Function interface to the Execution interface

isHA, hasResult, and optimizeForWrite are methods on Function that the user can override. However, that makes it harder to write lambda expressions for functions, because if you want to set one of these parameters you can now longer use a lambda.

...

See

The Function interface should return a value

The function context has a ResultSender with methods like sendResult, lastResult, sendException, etc. Unfortunately, that makes the common case of a function that returns a single result unnecessarily verbose and error prone. For example, if no result is sent that can cause issues:

...

interface RegionFunction<T> {
  public T execute(RegionFunctionContext context)
}

interface MemberFunction<T> {
  public T execute(FunctionContext context)
}

Combining these changes with

interface RegionFunction<S, T> {
  public T execute(RegionFunctionContext<S> context)
}

interface MemberFunction<S, T> {
  public T execute(FunctionContext<S> context)
}

interface FunctionContext<S> {
  public S getArguments();
}

See

Add Cache.getFunctionService and deprecate the static methods on FunctionService

This proposal might be a bit more controversial. The issue is that the current FunctionService leads people to write code that is harder to test, because they cannot mock the FunctionService if they are making static calls to FunctionService.onRegion. If FunctionService is something that could be provided by the cache, a user can mock FunctionService and assert that functions are invoked.

The other issue this fixes is that FunctionService has methods like onMembers(String ... groups) that internally look up a static cache. Besides making it hard for users to use Mocks with FunctionService code, this also ties us to having a singleton cache and distributed system. Since we're trying to get rid of singletons we should fix this API to not rely on an underlying singleton.

Stop Requiring the user to use the ResultCollector returned by the execute method.

The javadocs for execute say this

...

We should make sure that users don't have to call getResults on the returned ResultCollector. If we want to make it easier for users to implement a ResultCollector that waits for all of the results to arrive, we should provide helper classes like a BlockingResultCollector that waits for all results to arrive in getResults.

Part of the reason is is required right now is the way that exceptions get handled. If a function throws an exception or even just sends an exception with ResultSender.sendResult, the user's ResultCollector will never see it. The only way to get the exception is to call getResult on the result collector returned by execute. See

Summary of changes

New interfaces

interface RegionFunction<S, RegionFunction<T>T> {
  public T execute(RegionFunctionContextRegionFunctionContext<S> context)
}

interface MemberFunction<T>MemberFunction<S, T> {
  public T execute(FunctionContextFunctionContext<S> context)
}

interface RegionExecution<T,S>RegionExecution<S, T, U> extends Execution {
  public ResultCollector<T, S>U> execute(RegionFunction<T>RegionFunction<S,T> function);
}

interface MemberExecution<T,S>MemberExecution<S, T, U> extends Execution {
  public ResultCollector<T, S>U> execute(MemberFunction<T>MemberFunction<S,T> function);
}

New Methods

FunctionService Cache.getFunctionService()

Region RegionFunctionContext.getLocalDataSet();

Execution.isHA(boolean value)
Execution.optimizeForWrite(boolean value)
Execution.hasResult(boolean value)

//New instance methods, rather than static
FunctionService {
  public RegionExecution onRegion(Region region);
  public MemberExecution onServer(RegionService regionService);
  public MemberExecution onServers(RegionService regionService);
  public MemberExecution onMember(DistributedMember distributedMember);
  public MemberExecution onMembers(Set<DistributedMember> distributedMembers);
  public MemberExecution onMember(String... groups);
  public void registerFunction(Function function);
  public void unregisterFunction(String functionId);
  public boolean isRegistered(String functionId);
}

Deprecated Methods

FunctionService {
  public static Execution onRegion(Region region);
  public static Execution onServer(Pool pool);
  public static Execution onServers(Pool pool);
  public static Execution onMember(DistributedMember distributedMember);
  public static Execution onMember(String... groups);
  public static Function getFunction(String functionId);
  public static void registerFunction(Function function);
  public static void unregisterFunction(String functionId);
  public static boolean isRegistered(String functionId);
}

Function {
  public boolean hasResult();
  public boolean optimizeForWrite();
  public boolean isHA();
}

Examples of API changes

FunctionService functionService = cache.getFunctionService();

//RegionFunction with a single result
ResultCollector<String, Collection<String>> rc = functionService.onRegion(region)
  .execute(ctx -> ctx.getLocalData().get("key"))
Collection<String> values = rc.getResult()

//RegionFunction without a result, that is optimized for write, but is not HA
functionService.onRegion(region)
  .optimizeForWrite(true)
  .isHA(true)
  .hasResult(false)
  .execute(ctx -> ctx.getLocalData().put("key", "value))

//On member function that gets the cache from the context
functionService.onMembers()
   .hasResult(false)
   .execute(ctx -> ctx.getCache().getName());

//Function that streams results back using the result sender
functionService.onRegion(region).execute( ctx -> 
  {
    ResultSender<String> s = ctx.getResultSender();
    for(String value : ctx.getRegion().values()) {
      s.addResult(value)
    }
    return null;
  }