Cache fetched apps in update check

[MorrisJobke]

The code tried to find the apps with updates and thus was called for every available app. This caused to get the full appstore content as often as apps are available. The appstore request itself was cached nevertheless in an appdata dir, but with an object storage this is still a lot of round trips to read this cached result. Thus the instantiated list is now cached in a static variable (because it's a static method call).

Fixes #7235

Diff of the before and after code base: https://blackfire.io/profiles/compare/0b59e81d-4ea4-4a22-ba14-640cf091d487/graph

[nickvergessen]

Static.... ⚱️

[nickvergessen]

Actually it should be cached already?

server/lib/private/App/AppStore/Fetcher/Fetcher.php

Lines 143 to 153 in 2eb2b6e

	// File does already exists
	$file = $rootFolder->getFile($this->fileName);
	$jsonBlob = json_decode($file->getContent(), true);
	if (is_array($jsonBlob)) {
	// No caching when the version has been updated
	if (isset($jsonBlob['ncversion']) && $jsonBlob['ncversion'] === $this->getVersion()) {
	// If the timestamp is older than 300 seconds request the files new
	if ((int)$jsonBlob['timestamp'] > ($this->timeFactory->getTime() - self::INVALIDATE_AFTER_SECONDS)) {
	return $jsonBlob['data'];

[nickvergessen]

Ah reading helps...

[nickvergessen]

There was 1 failure:

1) Test\InstallerTest::testIsUpdateAvailable with data set #1 (array(array('files', array(array('1.0')))), false)
Failed asserting that '1111.0' is identical to false.

/drone/src/github.com/nextcloud/server/tests/lib/InstallerTest.php:157

Static fun

[MorrisJobke]

Static fun

And I have no idea what this is :/

[MorrisJobke]

And I have no idea what this is :/

Ah - because it is cached and not cleared between those jobs :/

[codecov]

Codecov Report

Merging #7264 into master will increase coverage by 0.02%.
The diff coverage is 29.72%.

@@             Coverage Diff              @@
##             master    #7264      +/-   ##
============================================
+ Coverage     50.85%   50.88%   +0.02%     
- Complexity    24551    24552       +1     
============================================
  Files          1585     1585              
  Lines         93833    93791      -42     
  Branches       1354     1354              
============================================
+ Hits          47719    47725       +6     
+ Misses        46114    46066      -48

Impacted Files	Coverage Δ	Complexity Δ
core/ajax/update.php	0% <0%> (ø)	11 <0> (ø)	⬇️
settings/ajax/updateapp.php	0% <0%> (ø)	0 <0> (ø)	⬇️
lib/base.php	3.09% <0%> (-0.04%)	167 <0> (ø)
core/Command/Maintenance/Install.php	0% <0%> (ø)	21 <0> (ø)	⬇️
core/Command/Upgrade.php	0% <0%> (ø)	28 <1> (ø)	⬇️
core/Command/App/Install.php	0% <0%> (ø)	5 <0> (ø)	⬇️
core/register_command.php	0% <0%> (ø)	0 <0> (ø)	⬇️
lib/private/Setup.php	12.76% <10%> (+0.42%)	50 <0> (ø)	⬇️
lib/private/Updater.php	6.6% <20%> (+0.41%)	77 <0> (ø)	⬇️
lib/private/Server.php	82.62% <25%> (-0.69%)	126 <1> (ø)
... and 14 more

[MorrisJobke]

Ah - because it is cached and not cleared between those jobs :/

Okay ... I needed to take a bigger reordering of how it works. basically the second commit in here brings the DI version to all places where the Installer is used. And the third commit replaced the static calls to isUpdateAvailable to normal method calls on the Installer object. I tested around and it works fine (enabling/disabling/updating apps).

@nickvergessen @rullzer @LukasReschke Please review

[blizzz]

Several tests are failing, but on first sight they seem unrelated… but those happen not on master.

[MorrisJobke]

Several tests are failing, but on first sight they seem unrelated… but those happen not on master.

It was related. I forgot to mock the call in the tests, so it asked for actual updates in the appstore. 🙈

[MorrisJobke]

Obviously I failed here too. 🙈 cc @rullzer @skjnldsv Let me fix that.