Fixes #6360: Optional dependency on REST manager in components

[kozlovsky]

This PR makes the dependency of components on REST manager optional. It adds a base class RestfulComponent, which provides three methods:

The init_endpoints method simplifies endpoint initialization and can be used inside a component's run method to inject values to REST endpoints:

async def run(self):
    ...
    await self.init_endpoints(
        ["endpoint_name1", "endpoint_name2"]
        [(attr_name1, value1), (attr_name2, value2)]
    )

The method sets specified attributes to all enlisted endpoints. If RESTComponent or some specific endpoint is not available, it just skipped silently.

It also is possible to initialize ipv8 endpoints as well by specifying the ipv8 instance.

        await self.init_ipv8_endpoints(self._ipv8, ['tunnel'])

This way it becomes not necessary to explicitly access RESTComponent and REST endpoints in a component's code.

The shutdown method of RestfulComponent will release endpoints by replacing previously set values to None:

The set_readable_status abstracts out the setting Tribler user-readable status to a specific value.

async def run(self):
    ...
    await self.set_readable_status(STATE_START_WATCH_FOLDER)

[kozlovsky]

retest this please

[kozlovsky]

retest this please

[drew2a]

I appreciate your care about developers in terms of "encapsulating logic to a base class" and simplifying the code of components.

I am just afraid that it adds cognitive complexity to the components themselves.

Now operations with REST are not as obvious as they were before.

I would suggest just making a dependency of RESTComponent optional (with adding the corresponding code) without incapsulating it in a base class.

[kozlovsky]

I appreciate your care about developers in terms of "encapsulating logic to a base class" and simplifying the code of components.

I am just afraid that it adds cognitive complexity to the components themselves.

Now operations with REST are not as obvious as they were before.

I would suggest just making a dependency of RESTComponent optional (with adding the corresponding code) without incapsulating it in a base class.

@drew2a in general I agree that inheritance adds some cognitive complexity, and try to avoid using it.

But in this specific case I think that having the logic of endpoint initialization repeated in many components is a bit cumbersome for the following reason:

1. It is possible, that RESTComponent is unavailable.
2. Even if RESTComponent if present, it is possible that some specific subset of rest endpoints which a component is trying to initialize is disabled, so it is necessary to check availability of some endpoint before initialization
3. During the shutdown we again need to skip cleanup of endpoints which were disabled.

For example, this fragment of TunnelComponent initialization code needs to check the endpoints availability:

        rest_component = await self.get_component(RESTComponent)
        if rest_component:
            rest_component.rest_manager.get_endpoint('ipv8').endpoints['/tunnel'].initialize(self._ipv8)
            if download_component:
                rest_component.rest_manager.get_endpoint('downloads').tunnel_community = community

            debug_endpoint = rest_component.rest_manager.get_endpoint('debug')
            debug_endpoint.tunnel_community = community

Now it looks like this:

    await self.init_endpoints(['downloads', 'debug'], [('tunnel_community', community)])
    await self.init_ipv8_endpoints(self._ipv8, ['tunnel'])

In my opinion, providing reusable methods for endpoint initialization makes the remaining code of components easier to understand.

[kozlovsky]

retest this please

[drew2a]

@kozlovsky you are right, agree.

By the way, to increase readability we may use named arguments, eg:

    await self.init_endpoints(endpoints = ['downloads', 'debug'], values = [('tunnel_community', community)])
    await self.init_ipv8_endpoints(self._ipv8, endpoints = ['tunnel'])

or even use a dict instead a list of tules:

    await self.init_endpoints(endpoints = ['downloads', 'debug'], values = {'tunnel_community': community})
    await self.init_ipv8_endpoints(self._ipv8, endpoints = ['tunnel'])

[ichorid]

@kozlovsky you are right, agree.

By the way, to increase readability we may use named arguments, eg:

    await self.init_endpoints(endpoints = ['downloads', 'debug'], values = [('tunnel_community', community)])
    await self.init_ipv8_endpoints(self._ipv8, endpoints = ['tunnel'])

or even use a dict instead a list of tules:

    await self.init_endpoints(endpoints = ['downloads', 'debug'], values = {'tunnel_community': community})
    await self.init_ipv8_endpoints(self._ipv8, endpoints = ['tunnel'])

@drew2a is right. Just use a single, dict argument.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[kozlovsky]

@drew2a @ichorid I changed arguments according to your suggestion. I agree that the dict argument is more appropriate here.