Check LastVersion of third party modules ONCE PER SESSION [one line solution PROPOSED]

caos30 commented 2 months ago

Feature Request

Hello, I have already developed 7 modules for Dolibarr (published on Dolistore) and in all of them I have defined the url_last_version property in the module's definition file modMymodule.class.php.

This causes Dolibarr to CHECK if there's a new version of my modules every time the user visits ANY OF THE TABS in the "Setup/Modules" section... which usually takes about 40-60 seconds each time!! This makes it "almost impossible" to work on module configuration and activation, really.

This will worsen as more developers discover that this is possible. Honestly, I think it's something very useful for the users of my modules. But it can't work this slowly.

Use case

No response

Suggested implementation

My improvement proposal, which I've already tested in Dolibarr 20, requires changing one line in the /core/modules/DolibarrModules.class.php file to STORE IN THE USER'S SESSION VARIABLE the last successful query to the REMOTE URL, and in subsequent calls to this function, that value is used for as long as the user session lasts.

In PHP code it would be:

File: /core/modules/DolibarrModules.class.php Method: public function checkForUpdate() Current code:

$lastVersion = getURLContent($this->url_last_version, 'GET', '', 1, array(), array('http', 'https'), 0);    // Accept http or https links on external remote server only

New code:

if (!empty($_SESSION['lastVersion_'.$this->name])) {
    $lastVersion =  $_SESSION['lastVersion_'.$this->name];
} else {
    $lastVersion = getURLContent($this->url_last_version, 'GET', '', 1, array(), array('http', 'https'), 0);    // Accept http or https links on external remote server only
    if (isset($lastVersion['content'])) $_SESSION['lastVersion_'.$this->name] = $lastVersion;
}

Thanks in advance.

Note: i've not added a pull request because it is not really a "bug". So i think that main developers should read my proposal and approve it or even improve it.

Suggested steps

No response

clementgde commented 2 months ago

It's not a bug, but it's very annoying for the user experience. In addition, I'd like to add a few closely related points: You should add the possibility of checking whether the url exists before making the request, because the 404 not found messages I've been getting for 5 years because I have old modules are a pain. Finally, on the same level, I think the messages should be more discreet, perhaps on the page intended for the module itself rather than on the general modules page. I'm a developer and I spend my life on this page, and it's a real pain to see “A new version is available” all the time, especially as it's not even major versions, it's 0.2 that become 0.3.

caos30 commented 2 months ago

You should add the possibility of checking whether the url exists before making the request, because the 404 not found messages I've been getting for 5 years because I have old modules are a pain.

It's impossible "to check if a URL exists before trying to download it"... The only way to know that a remote URL doesn't exist is to try to download it 😅
Having said this, I perfectly understand the frustrating experience of waiting every time you visit the Modules Setup page to check the download of this non-existing URL! So I agree 100% with you that we should take a monitoring action.
The only solution I can think of is to build a count of the times/days that this URL is not responding to the request. Then, after exceeding a threshold number of attempts, we should disable it. There are various possible ways to do this.
For counting failed attempts, I would implement 2 measures:
- Save the fail on user SESSION (for example a simple: $_SESSION['attemptsURL_'.$this->name]). This way, we would avoid making this remote URL retrieval more than X times per session (2 times? 1 time?)
- Save failed attempts in the database on different DAYS. This way, we would be able to know, for example, that we have failed to contact that URL for X days. The counting would be reset when the request succeeds. But doing this count would probably require a new field in the database. Or maybe store that info in a physical file in the /custom/ThirdPartyModule/ directory (if writable)
When the number of failed attempts exceeds the defined threshold (X days failing), then we need to disable this action forever. Here too, there are several ways:
- Certainly, if we have created a field in the database to store the retrieval attempts, it would be enough to check that number of failed attempts and then decide whether to make the request to the remote URL or not.
- A more "crude" way would be to do a PHYSICAL REMOVAL of the URL in the file modThirdPartyModule.class.php using a simple preg_replace() action on the content of that file, to convert https://ThirdParty.com/ThirdPartyModule/... into a blank string '' or also into something like __https://ThirdParty.com/ThirdPartyModule/.... Then, when we detect that the URL doesn't begin with http, the curl request will not be executed. This way, we would keep the original URL, to be able to manually check it later.

clementgde commented 2 months ago

To avoid 404 errors, you could ping the url and record the errors.

caos30 commented 2 months ago

To avoid 404 errors, you could ping the url and record the errors.

Interesting approach, but this would only partially and occasionally address the problem because:

a PING is a call to a server (IP address, in fact),
so a PING call cannot determine if a specific resource (file, script, etc.) exists on the server.
however, this could be helpful before making the curl() call to verify that the domain name of the URL still exists.
if the domain name exists but the full path to the resource does not, the PING would not prevent the long timeout while waiting for the response

Dolibarr / dolibarr