Implement listener level interceptors using createInterceptors function.

[dilanSachi]

Description:

import ballerina/http;

service class MyRequestInterceptor {
    *http:RequestInterceptor;

	resource function 'default [string... path](http:RequestContext ctx) returns http:NextService|error? {
		return ctx.next();
	}
}

service class MyResponseInterceptor {
    *http:ResponseInterceptor;

	remote function interceptResponse(http:RequestContext ctx) returns http:NextService|error? {
		return ctx.next();
	}
}

// Ideally, this will be a part of `http` module
public type CreateInterceptorsFunction function () returns http:Interceptor[];

// The function implementation. Note the tuple return type.
function createInterceptors() returns [MyResponseInterceptor, MyRequestInterceptor] {
    return [new MyResponseInterceptor(), new MyRequestInterceptor()];
}

// This class is just to mock the `http:Listener`
public class Listener {
	http:Interceptor[] interceptors;
	int port;

	function init(int port, CreateInterceptorsFunction interceptors) {
		self.port = port;
		self.interceptors = interceptors();
	}
}

Listener serverEP = new(9090, interceptors = createInterceptors);

For the discussion see #4300

[dilanSachi]

• Add runtime support for the change
• Add compiler level deprecations
• Update BBEs and samples

[dilanSachi]

The issue here when adding a new parameter for the http:Listener initialization is it may break the existing users.

public isolated function init(int port, CreateInterceptorsFunction? interceptorsFunction, *ListenerConfiguration config)
            returns ListenerError? {

As a solution we can add the interceptorsFunction as a field in http:ListenerConfiguration.

public type ListenerConfiguration record {|
    ...
    # interceptors - An array of interceptor services
    # # Deprecated
    # Defining interceptor pipeline in `http:ListenerConfiguration` is deprecated. Define the interceptor pipeline
    # via `http:InterceptableService` service type
    @deprecated
    Interceptor|Interceptor[] interceptors?;
    ...
    CreateInterceptorsFunction interceptorsFunction?;
|};

However this may introduce some complexities when giving compiler errors to the user if the user has defined the ListenerConfiguration record elsewhere. In that case we can provide a runtime error as well to the user however.

@shafreenAnfar @TharmiganK WDYT?

Any suggestions apart from above?

[TharmiganK]

We can add the default value as nil for interceptorsFunction right? Then it won't be a breaking change

public isolated function init(int port, CreateInterceptorsFunction? interceptorsFunction = (), *ListenerConfiguration config)
            returns ListenerError? {

However this may introduce some complexities when giving compiler errors to the user if the user has defined the ListenerConfiguration record elsewhere. In that case we can provide a runtime error as well to the user however.

This can occur for any of these options. It is hard to give a compiler plugin warning/error here. We might have to add a runtime warning/error.

There is another option to wrap the existing http:Listener into a http:InterceptableListener like this:

public isolated class InterceptableListener {

    private final http:Listener httpListener;

    public isolated function init(int port, CreateInterceptorsFunction interceptors,
            *record{|*http:ListenerConfiguration; never interceptors?;|} config) 
        returns http:ListenerError? {
        
        self.httpListener = check new(port, { ...config, interceptors: interceptors()});
    }
    
    ...
}

The advantage here is we have restricted the users to define the interceptors in either one of the way. So they cannot have both.

[dilanSachi]

We can add the default value as nil for interceptorsFunction right? Then it won't be a breaking change

No right?
For a old listener initialization as follows,

http:Listener ls = check new (5005, "localhost");

This will give an error as incompatible types: expected 'ballerina/http:2.7.1:CreateInterceptorsFunction?', found 'string' right

[dilanSachi]

Also another option is changing interceptors field from Interceptor|Interceptor[] interceptors?; to Interceptor|Interceptor[]|CreateInterceptorsFunction interceptors?;

This way users cannot define both at the same time. But we may have to give runtime warnings for the deprecation (however the field name does not match with the type as well).

[TharmiganK]

We can add the default value as nil for interceptorsFunction right? Then it won't be a breaking change

No right? For a old listener initialization as follows,

http:Listener ls = check new (5005, "localhost");

This will give an error as incompatible types: expected 'ballerina/http:2.7.1:CreateInterceptorsFunction?', found 'string' right

Yes, it will break if we have given the config map directly as the second parameter.

So we have three options here:

Options	Pros	Cons
Have two different fields in the ListenerConfiguration and deprecate one	- Meaningful separation	- Need to add a runtime error when the two fields are used - Connectors need to update their configurations
Change the interceptors type to be union with CreateInterceptorsFunction	- Can only use one way to define the interceptor	- Field name does not match with the type - Connectors need to update their configurations
Wrap http:Listener into http:InterceptableListener	- Can only use one way to define the interceptor	- Connectors need to use http:InterceptableListener instead if they are having interceptors - Abstraction is not perfect since the http:Listener anyway have the DefaultInterceptor

[dilanSachi]

Decided go ahead with option 2

[dilanSachi]

Decided to stop supporting interceptors in listener level.

1. Even if we provide the interceptors as a function, it is difficult for openapi to read the types from that.
2. We can do the same things we do by using service level interceptors. (However there are some challenges in this).
   • To handle not found cases when there are multiple service attached. Decided to resolve this by needing to add a root level (/) interceptor.