GH-1387 Improved custom service executor extension system

[Aklakan]

GitHub issue resolved #1387.
This fixes #1399.

Pull request Description: This PR adds the following changes to the custom service executor plugin system:

Improvements:

• chaining There is now ChainingServiceExecutor which allows for transparently forwarding a request to the next ChainingServiceExecutor instance in the registry. This way a custom service executor can modify the request and have it processed by the remainder of the chain.
• bulk processing The plugin system introduced with jena 4.5.0 only supports lookups with individual bindings. This PR adds support for plugins that want to process bindings in bulk. The main difference in the APIs is whether the createExecution method accepts a single Binding or a QueryIterator. There is now a ServiceExecutorRegistryBulk which by default has an registration that delegates to the non-bulk one.

Breaking changes:

• ServiceExecutorRegistry.getFactories() now returns a List<ChainingServiceExecutor> because this is its internal storage; previously it was List<ServiceExecutorFactory>. I am afraid re-establishing compatibility in this regard would require a completely new registry with a new context attribute.

Deprecations:

• Deprecated ServiceExecutorFactory in favor of ServiceExecutor (ServiceExecutorFactory extends from the latter). Use of the old code delegates to the new one.
  • In the new code the method that creates a ServiceExecution is now called createExecution (rather than createExecutor)
• Deprecated ServiceExecutorRegistry.add in favor of prepend. Both methods add executors to the beginning of the registry's list so they are considered before the default behavior. QueryEngineFactory also uses add to prepend; so add is consistent with existing naming.
• Legacy add method for ServiceExecutorFactory exists and wraps it as a ChainingServiceExecutor.
• Legacy remove method for ServiceExecutorFactory finds previously wrapped elements..
• Deprecated ServiceExecution because it doesn't add anything over QueryIterator.

---

• Javadoc exists and is up-to-data
• Tests are included.
• Documentation change and updates are provided for the Apache Jena website
• Commits have been squashed to remove intermediate development commit messages.
• Key commit messages start with the issue number (GH-xxxx or JENA-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.

---

See the Apache Jena "Contributing" guide.

[afs]

Please explain what is going on here.

If a change is needed, then having a superclass is not necessary. Just two nextStage methods with different signatures.

[Aklakan]

QueryIterApplyBulk's nextStage(QueryIterator) method flat-maps the input QueryIterator to an output QueryIterator until the input iterator is consumed - i.e. implementations of nextStage can "per stage" consume an arbitrary amount > 0 from the input iterator.

QueryIterApply implements QueryIterApplyBulk.nextStage(QueryIterator) such that it delegates to its own usual QueryIterApply.nextStage(Binding) method.
In other words, QueryIterApplyBulk is all methods of QueryIterApply minus this one delegate.

Note that QueryIterApplyBulk is also the base class for QueryIterServiceBulk. The latter only operates on the QueryIterator and therefore does not need a nextStage(Binding) method. I find this separation cleaner than having both nextStage methods on the same class together with an override that causes the binding-level one to no longer be called. Other than that, having both methods on the same class would work too.

The only drawback I see with this separation is that reflection code that relied on QueryIterApply.class.getDeclaredMethods() would break.

[afs]

I find this separation cleaner

My concern is that this is making the bulk case "normal" when in fact it is special to your service work.

[Aklakan]

Just to clarify, my next step on this PR is to update and write javadoc; especially for what purpose some of the classes are for. This should make reviewing easier.

[Aklakan]

May latest attempt at cleaning up and simplifying things:

• Reverted QueryIterRepeatApply (it used inside of ServiceExecutorBulkToSingle which acts as the bridge between bulk and non-bulk)
• Combined bulk and non-bulk registries
• Deprecated ServiceExecution in favor using QueryIterator directly
• OpExecutor now directly calls the plugin system
• This way QueryIterService is actually not used anymore - service execution via the registry eventually by default ends up with a QueryIterRepeatApply. The still existing QueryIterService only uses the non-bulk part of the registry.

If there should be a dedicated QueryIterService which calls the registry rather than OpExecutor doing that, then actually the proper base class would be something like a QueryIterDeferred / LazyInitialization: That iterator just takes the iterator returned by the service handler and serves from it. No repeat apply / batching / stages needed at that time.

[afs]

This change isn't necessary but if it is made, please use IteratorCloseable See the comments in ClosableIterator which refer to the Model API and ExtendedIterator - let's keep ClosableIterator for jena-core.

[Aklakan]

Updated. It seemed reasonable to add one of the interfaces that already combined iterator with close - it was a 50/50 chance 😄

[afs]

This is now an unused import and can be removed.

[Aklakan]

I removed the import and added a deprecation annotation because QueryIterService is now no longer used but existing code may have sub-classed from it.

[afs]

Looks good - 2 small comments.

Then it's the checklist items : javadoc item and how you want to commits to look when merged to the codebase. e.g any squashing and and any use of "GH-1387: " to pick out key commits.

[Aklakan]

I noticed that the overloaded add methods in ServiceExecutorRegistry would result in ambiguity with lambdas - which broke with some of my code based on the 4.5.0 api.
Before the naming gets frozen I now renamed the methods to:

add(ServiceExecutor) // Compatible with 4.5.0
addSingleLink(ChainingServiceExecutor) // New method; adds a link to the chain
addBulkLink(ChainingServiceExecutorBulk) // Also new

I updated javadoc and added a couple of @deprecated(since = "4.6.0") annotations.
I hope its final now.

[Aklakan]

Squashed

[afs]

Are there any documentation changes for jena-site?

Also - 2 or 3 sentences for the 4.6.0 release announcement would be good.