Usability issues:

Must use ResultCollector returned from FunctionService.execute

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?

Example:

  ResultCollector<TopEntriesCollector, TopEntries> rc = (ResultCollector<TopEntriesCollector, TopEntries>) FunctionService.onRegion(region)
        .withArgs(context)
        .withCollector(collector)
        .execute(LuceneFunction.ID);
    
//This doesn't work
TopEntries entries = collector.getResult()

//This is what you have to do
TopEntries entries = rc.getResult();
  

Exceptions are handed to ResultCollector.addResult, causing ClassCastExceptions

On the ResultSender, there is a method to return exceptions using sendException.

However, the ResultCollector only has addResult(). Further, ResultCollector has generic types

public interface ResultCollector<T,S> {
  public void addResult(DistributedMember memberID, T resultOfSingleExecution);

Since the type of T is the user result type, trying to call add result will almost invariably result in the ClassCastException, like below

[fatal 2015/12/03 10:58:20.809 PST <Function Execution Processor1> tid=Function Execution Processor1id] Unexpected exception during function execution on local node Partitioned Region
java.lang.ClassCastException: java.lang.Exception cannot be cast to com.gemstone.gemfire.cache.lucene.internal.distributed.TopEntriesCollector
    at com.gemstone.gemfire.cache.lucene.internal.distributed.TopEntriesFunctionCollector.addResult(TopEntriesFunctionCollector.java:1)
    at com.gemstone.gemfire.internal.cache.execute.LocalResultCollectorImpl.addResult(LocalResultCollectorImpl.java:85)
    at com.gemstone.gemfire.internal.cache.execute.PartitionedRegionFunctionResultSender.lastResult(PartitionedRegionFunctionResultSender.java:216)
    at com.gemstone.gemfire.internal.cache.execute.PartitionedRegionFunctionResultSender.lastResult(PartitionedRegionFunctionResultSender.java:174)
    at com.gemstone.gemfire.internal.cache.execute.PartitionedRegionFunctionResultSender.sendException(PartitionedRegionFunctionResultSender.java:309)
    at com.gemstone.gemfire.cache.lucene.internal.distributed.LuceneFunction.execute(LuceneFunction.java:60)
    at com.gemstone.gemfire.internal.cache.execute.AbstractExecution.executeFunctionLocally(AbstractExecution.java:367)
    at com.gemstone.gemfire.internal.cache.

onMember functions cannot be unit tested, because our API requires using a singleton cache

FunctionContext does not contain a cache. That means any function that you execute with onMember must contain code like this:

public void execute(FunctionContext context) {
    Cache cache = CacheFactory.getAnyInstance();
//...do work with the cache
}

This makes it impossible to write pure unit tests for the function code that mock the cache.

onRegion functions always have to cast the FunctionContext to RegionFunctionContext

Any function that is executed on a region has to do a cast. 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

All onRegion function code basically has to start this way:

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

Getting the local data set of a function is not intuitive, uses static functions that expect concrete objects

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

   public void execute(FunctionContext context) {
      RegionFunctionContext ctx = (RegionFunctionContext) context;
      //This actually doesn't give you the local data set
      Region wholePartitionedRegion = ctx.getDataSet();

      //This does, using a static function call 
      Region localData = PartitionRegionHelper.getLocalDataForContext(ctx);

      //This is a bad idea, because it may include some buckets that are also being processed on other nodes
      Region localDataDontDoThis = PartitionRegionHelper.getLocalData(ctx.getDataSet());

There are several problems with this approach.

• 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