Add Request class to standardise request interface

[st0012]

Motivation

Instead of having some request classes implementing run method as interface, and other listener-based classes implementing response instead, we should standardise the interface to just response.

Since I made a similar change in #1247 as well, merging this first will also make it easier to review.

Implementation

We can enforce the interface by making all request classes to inherit an abstract Request class.

Note: after doing a major refactor in #1247, the only commonality shared between all request classes is that they all implement a method to return the result. I don't think there's more that we could put into the Request class atm.

Automated Tests

All existing tests should pass.

Manual Tests

[egiurleo]

Really cool how the architecture and patterns in this codebase are consolidating over time 😀

[vinistock]

What do you think about standardizing requests with run instead of response?

In terms of communicating the intent, I think it makes sense to use run for requests and response for listeners. We run requests, it's the thing that executes them.

But for listeners, their execution is orchestrated by the dispatcher, so we don't run them. The dispatcher emits the events and then we just inspect the response.

Using response for requests results in

Request.new(...).response

Which at first glance makes me think like the initialize method is the thing running the request.

With the separation of requests and listeners to be separate entities, I'd still keep run for the request interfaces and response for the listeners.

What do you think?

[vinistock]

I think we can do

Suggested change

	klass if klass.ancestors.include?(Request) && !klass == Request
	klass if klass < Request && !klass == Request

[vinistock]

What do you think about standardizing requests with run instead of response?

In terms of communicating the intent, I think it makes sense to use run for requests and response for listeners. We run requests, it's the thing that executes them.

But for listeners, their execution is orchestrated by the dispatcher, so we don't run them. The dispatcher emits the events and then we just inspect the response.

Using response for requests results in

Request.new(...).response

Which at first glance makes me think like the initialize method is the thing running the request.

With the separation of requests and listeners to be separate entities, I'd still keep run for the request interfaces and response for the listeners.

What do you think?

[st0012]

Which at first glance makes me think like the initialize method is the thing running the request.

@vinistock It make sense. But if we change it before completely splitting Listener and Request, it'd be a big change as we'd need rename Listener#response too, which would affect non-request listeners as well.

[vinistock]

Chatted with Stan on a call. This is temporary step until the separation between requests and listeners is complete, just to ensure consistency while we get there.

All good, but still think we can change the ancestors check