...

Functions executed on a region should implement a different interface

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

public void execute(FunctionContext context) {
    RegionFunctionContext ctx = (RegionFunctionContext) context;
 

This is not an intuitive API; the user has to know that there is such a thing as RegionFunctionContext. This can also lead to ClassCastExceptions if the user tries to cast the FunctionContext and it is not actually a RegionFunctionContext

Instead, we should have a new RegionFunction interface that provides a RegionFunctionContext. FunctionService.onRegion should return a RegionExecution interface who's execute method should expect a RegionFunction.

See

RegionFunctionContext should provide the local data set

...

• It's not clear that ctx.getDataSet does not return the local data.
• The local data set is not mockable for unit tests, because this static function call extracts it using internal APIs on concrete classes. This also prevents this sort of function from being tested in a pure unit test.
• The user has to know that PartitionRegionHelper exists; the API is not obvious
• The various methods on PartitionRegionHelper are confusing, specifically it's unclear to a user why they should use getLocalDataForContext instead of getLocalData

Instead, RegionFunctionContext should just provide a getLocalDataSet method. We will add this new method to the RegionFunctionContext interface. We will deprecate the PartitionRegionHelper.getLocalDataForContext.

Functions executed on a region should implement a different interface

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

public void execute(FunctionContext context) {
    RegionFunctionContext ctx = (RegionFunctionContext) context;
 

This is not an intuitive API; the user has to know that there is such a thing as RegionFunctionContext. This can also lead to ClassCastExceptions if the user tries to cast the FunctionContext and it is not actually a RegionFunctionContext

Instead, we will add a new RegionFunction interface that provides a RegionFunctionContext. FunctionService.onRegion will return a RegionExecution interface with an execute method that takes a RegionFunction.

See

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

...

Also the function service API provides a way to invoke functions using a string id. For example FunctionService.onServer().execute("MY_FUNCTION_ID"). If we remove these extra methods from the Function interface, this code will no longer need to look up the Function on the client and the Function class will not have to exist on the client anymore. any more.

We will add isHA(boolean), hasResult(Boolean) and optimizeForWrite(boolean) to the Execution interface. These methods will be deprecated on the function interface. Unfortunately, if someone invokes a function on a client given the string id, we will still need to look up the function class for now in case they have specified these parameters on their function. But in another release we can remove the deprecated parameters and stop using them.

The Function interface should return a value

...

public void execute(FunctionContext context) {
  if(someTest) {
	  context.getResultSender().lastResult("Done");
  }
  //Whoops I forgot to send a last result
}

...

Instead, the function interfaces should return a value. For the simple case where there is only one result, the user can just return the result. For cases with multiple streaming results, the user can look up the result sender. For cases with no results the user can just return null. The return value will be ignored anyway if the user invokes the function with hasResult==false.

We can't change the existing Function interface, but we can add a return value to the new RegionFunction interface we're introducing. We can also introduce a MemberFunction interface that returns a value, to distinguish it from the RegionFunction.

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

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

...

It's confusing to users that geode wraps their result collector in another collector that has different behavior, and it's unclear why the user has to call the returned result collector. We should stop wrapping the user's result collector, and instead provide helper classes like a BlockingResultCollector that waits for all results to arrive in getResults.

Below, we have to use the collector returned by FunctionService.execute, rather than just calling getResult on our own result collector. We run into problems otherwise, maybe because exceptions are not handed to the user supplied result collector? In any case, its unintuitive that this returns a different ResultCollector than the one supplied by the user.

Example:

...

.

...

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().closegetName());

//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;
  }