Remove special case for simpleAPI

[raintonr]

simpleAPI can exist as a standalone process, or a web adapter extension or... well... so what is the point of this:

    // Activate integrated simple API
    if (settings.simpleapi) {
        adapter.log.debug('Activating simple API');
        try {
            const SimpleAPI = require(utils.appName + '.simple-api/lib/simpleapi.js');

            server.api = new SimpleAPI(server.server, {secure: settings.secure, port: settings.port}, adapter);
        } catch (e) {
            adapter.log.error('Cannot find simple api module! ' + e);
        }
    }

I guess this is here for legacy reasons but IMHO simpleAPI should not be treated differently from any other adapter/extension.

Remove the above and associated field from the admin page.

[Apollon77]

No, in fact it is a "feature" of the web adapter to have a build in simple-api without the need to have the other adapter installed :) Changing this is a big breaking change

[raintonr]

Ok.

Just for my understanding, what's the different between the simpleAPI included in the web adapter and the standalone simpleAPI adapter?

If they do the same (or very similar) things strikes me as a bad idea to duplicate.

PS. I don't doubt it would be a breaking change, but sometimes you have to break legacy stuff to clean a system up.

[Apollon77]

I think the path is different but need to check ;-)

[Apollon77]

@GermanBluefox what do you think?

[raintonr]

I brought this up because noticed that a load of adapters were updated with the last LE changes. The admin settings for each of these adapters ask the same questions (port, certs or LE settings, etc). If there was some kind of plugable admin settings then possibly not that bad (ie. write the port, certs or LE settings, etc. part once and plug this in to each adapter that requires these) but not sure this exists in IOB.

So currently there seems to be a lot of duplicate code which seems a pain to maintain and I question for what gain?

IMHO, if an adapter needs LE (or if it needs to respond to HTTP(S) requests in general) it should be implemented as an extension to a web adapter. If that was the case then any future upgrades to LE would not affect this (the web adapter <-> extension interface would hide all that). It would also mean all that duplicate admin would go away in favour of a simple drop down to select which web adapter instance to run within.

The downside would be that the above isn't as modular as having each adapter fire up it's own Express server which could lead to path conflicts, but initially at least these could be solved with using multiple web adapter instances. What would the advantage of that to counterbalance the added config steps required to create multiple web adapter instances to house multiple extensions which required to be separate? At least the code would be cleaner (easier to maintain). Moreover, multiple, separate web adapters would only be required if added extensions either had a path conflict or were unstable - both situations we should strive to avoid and thus this really should be (or at least become) a non-issue.

In this scheme, simpleAPI wouldn't be any different and the legacy stuff that was coded for simpleAPI can go. That was my thought process at least.

Circling back round... maybe it is possible to have the current extension or standalone adapter implementation packaged up in a way there is one single place for code that houses a) the common admin settings - web adapter to extend or port, certs/LE settings and b) something akin to the web adapter <-> extension interface that would either allow the code to run inside the configured web adater or handle all the steps to fire up it's own Express server automatically. That could possibly be the best solution all round.

Thoughts?

[Apollon77]

Have anything "only" as web extension makes it more complex for the user because if you want to separate functionalities, also to "harden" the system you need to have multiple web instances and then only add one other thing to it, thats why the user can choose to open up a new server exactly for that one adapter OR add it to the web instance. it is "just" flexibility and less complexity in my eyes.

[raintonr]

In this case, I think we should look at the idea of having single codebase for the admin settings and function that can be called by any adapter implementing this to 'do stuff' based on those options. I am just trying to think of ideas to tidy the code, avoid duplication and make future maintenance easier is all.

*Not that this should detract from the original topic of this issue. simpleAPI already can be started standalone or in web adapter so really think the legacy special case hardcoded in web adapter should go.

[Apollon77]

I'm also not sure because itcould be that I WANT to have different configs per adapter because I maybe expose them with different domain names and such ... ;-) or?

[raintonr]

I'm also not sure because itcould be that I WANT to have different configs per adapter because I maybe expose them with different domain names and such ... ;-) or?

In that case you would be out of luck, because that is currently not supported. However, if the code was all shared you could implement additional settings once and all Express based adapters could benefit. See what I'm getting at? :D

[Apollon77]

Then it woud be an js-controller (or admin?) topic to provice something new centralized

[raintonr]

Then it woud be an js-controller (or admin?) topic to provice something new centralized

Are you guys open to going that direction or is it TooHard™ ;)

But either way, this legacy stuff could do with a clean up.

[GermanBluefox]

Let's encrypt is centralized and there are only 1 option to setup. On or Off. And only one instance (normally admin) has 3 options (On/Off, Update and port for http challenge)

I don't understand the problem. Is the one checkbox too much?

[raintonr]

As I said, "IMHO simpleAPI should not be treated differently from any other adapter/extension."

If you think differently then leave it as is and close this issue.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within the next 7 days. Please check if the issue is still relevant in the most current version of the adapter and tell us. Also check that all relevant details, logs and reproduction steps are included and update them if needed. Thank you for your contributions. Dieses Problem wurde automatisch als veraltet markiert, da es in letzter Zeit keine Aktivitäten gab. Es wird geschlossen, wenn nicht innerhalb der nächsten 7 Tage weitere Aktivitäten stattfinden. Bitte überprüft, ob das Problem auch in der aktuellsten Version des Adapters noch relevant ist, und teilt uns dies mit. Überprüft auch, ob alle relevanten Details, Logs und Reproduktionsschritte enthalten sind bzw. aktualisiert diese. Vielen Dank für Eure Unterstützung.

[GermanBluefox]

Built-in simple-api is the same (and use the same code as package) as simple-api itself. Yes, it should be rewritten as web-extension, but this integration appeared before web-extension and so exists. If you want, you can rewrite simple-api and web to be treated as web-ex, but now as there is no code duplication (but the same code as package), it is not critical.