Registering the `IInterceptorService` bean on `RestfulServer` results in duplicate interceptor calls

[MartinBernstorff]

The problem

If we use the bean registered in hapi-fhir-jpaserver-base here to construct the RestfulServer, any REST-request calls each interceptor twice.

The cause

I think this is because of the use of the composite broadcaster, e.g. here.

It receives the interceptors from:

• The request, which is populated in RestfulServer.newRequestDetails
• The DAO itself, which is autowired in BaseHapiFhirDao.myInterceptorBroadcaster

However, in this case, they end up being the same one!

Workaround

My understanding is that Interceptors are triggered like this:

Plain pointcuts

	Registered on
Call source	RestfulServer	IInterceptorService
REST	✅	🔴
DAO	🔴	🔴

Storage pointcuts

	Registered on
Call source	RestfulServer	IInterceptorService
REST	✅	✅
DAO	🔴	✅

✅: Triggered 🔴: Not triggered

Therefor, we maintain two different InterceptorServices, one for REST and one for non-REST.

Interceptors registered on RestfulServers InterceptorService are:

• Triggered on REST calls (non-storage pointcuts)
  • Because they are populated on the RequestDetails and BaseHapiFhirDao.myInterceptorBroadcaster combines both sources.
• Not triggered on storage pointcuts (DAO calls)
  • E.g. dao.update(myResource, new SystemRequestDetails())

Interceptors registered on the JPA InterceptorService bean are:

• Not triggered on non-storage pointcuts
  • Because we never hit the RestfulServer, the RequestDetails' interceptorservice is not populated
• Triggered on storage pointcuts (DAO calls)
  • If the pointcut is "storage"/"JPA server" (reference).

Leading us to the current heuristic: Any Interceptor that has storage hooks must be registered on the JPA InterceptorService, and any that has request hooks must be registered on the RestfulServer's InterceptorService.

No interceptor can have both types of hooks, because then they need to be registered in both places, and would be called twice.

However, this seems unnecessarily complicated.

What's next?

This leads me to a few questions:

• Why is there a need for two separate InterceptorServices? If we can eliminate this need, we eliminate the CompositeInterceptorBroadcaster, and therefor no duplicate calls.

  • One could imagine wanting to call a subset of interceptors only for REST calls. However, pushing that guarding into the interceptors themselves would radically simplify registration and the framework, and make the logic more explicit.

• Alternatively, would it make sense to check for interceptor identity in CompositeInterceptorBroadcaster, to avoid repeat calls?

If we really do need two interceptors:

• Document which interceptor to register on

• Throw/warn if registering interceptors on the wrong InterceptorService

Thanks a ton for the work you guys do!

[jamesagnew]

I don't hate the idea of reworking things to only have a single interceptor broadcaster instead of the current composite setup. It'd be a bunch of work though.

As it stands now, we don't support using the same service instance in both places.

[MartinBernstorff]

I don't hate the idea of reworking things to only have a single interceptor broadcaster instead of the current composite setup. It'd be a bunch of work though.

As it stands now, we don't support using the same service instance in both places.

Makes a lot of sense. Would a PR with documentation updates make sense? I.e. a "registering interceptors" section here, with an abbreviated version of the above?

[jkiddo]

Yes!