[locationinfo] Overhaul caching and prefetching of calendars

Prefetching of calendars now happens asynchronously in the background,
for all calendars that are still being displayed somewhere, and where a
certain age threshold is reached. This avoids slow and seemingly hanging
requests for updated calendar data, and for panels that are often
requested ad-hoc by users it increases chances that the calendar can be
served from cache.

9 files changed, 145 insertions, 105 deletions

diff --git a/modules-available/locationinfo/hooks/cron.inc.php b/modules-available/locationinfo/hooks/cron.inc.php
index 24d4c466..9f1ae36b 100644
--- a/modules-available/locationinfo/hooks/cron.inc.php
+++ b/modules-available/locationinfo/hooks/cron.inc.php
@@ -2,4 +2,6 @@
 
 $cutoff = time() - 86400 * 30;
 Database::exec('DELETE FROM locationinfo_backendlog WHERE dateline < :cutoff',
-	compact('cutoff'));
\ No newline at end of file
+	compact('cutoff'));
+
+LocationInfo::refreshStaleButActiveCalendars();
\ No newline at end of file
diff --git a/modules-available/locationinfo/inc/coursebackend.inc.php b/modules-available/locationinfo/inc/coursebackend.inc.php
index 95bef8ff..fe805b9c 100644
--- a/modules-available/locationinfo/inc/coursebackend.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend.inc.php
@@ -84,7 +84,7 @@ abstract class CourseBackend
 	}
 
 	/**
-	 * Get fresh instance of CourseBackend subclass for given backend type.
+	 * Get instance of CourseBackend subclass for given backend type.
 	 *
 	 * @param string $backendType name of module type
 	 * @return \CourseBackend|false module instance
@@ -134,8 +134,8 @@ abstract class CourseBackend
 	public abstract function getCacheTime(): int;
 
 	/**
-	 * @return int age after which timetables are no longer refreshed should be
-	 * greater then CacheTime
+	 * @return int time after which we consider a background async refresh of calendar that
+	 * is still actively being displayed somewhere
 	 */
 	public abstract function getRefreshTime(): int;
 
@@ -172,65 +172,62 @@ abstract class CourseBackend
 	}
 
 	/**
-	 * Method for fetching the schedule of the given rooms on a server.
-	 *
-	 * @param array $requestedLocationIds array of room ID to fetch
-	 * @return array array containing the timetables as value and roomid as key as result, or false on error
+	 * Method for fetching the schedule of the given locations on a server.
+	 * The returned array may not include all requested locations if there
+	 * was an error, or the calendar is empty.
+	 * @param array $requestedLocationIds array of location IDs to fetch
+	 * @return array array mapping the requested locationIDs to the according calendars, if found
 	 */
 	public final function fetchSchedule(array $requestedLocationIds, ?int $deadline): array
 	{
 		if (empty($requestedLocationIds))
 			return array();
+		// Special case: -1 means don't use cached schedules, fetch all.
+		// In that case, we also don't want to update the lastuse timestamp, as this is opportunistic. Updating
+		// the timestamp would make us fetch the schedule in the background forever, even if there is no browser
+		// open showing an according panel anymore.
+		$backgroundPrefetch = ($deadline === -1);
+		if ($backgroundPrefetch) {
+			$deadline = time() + 90;
+		}
 		$requestedLocationIds = array_values($requestedLocationIds);
 		$NOW = time();
-		$dbquery1 = Database::simpleQuery("SELECT locationid, calendar, serverlocationid, lastcalendarupdate
+		$locationData = Database::queryAll("SELECT locationid, calendar, serverlocationid, lastcalendarupdate
 				FROM locationinfo_locationconfig WHERE locationid IN (:locations)",
 				array('locations' => $requestedLocationIds));
 		$returnValue = [];
 		$remoteIds = [];
-		foreach ($dbquery1 as $row) {
-			// Check if in cache - if lastUpdate is null then it is interpreted as 1970
-			if ($row['lastcalendarupdate'] + $this->getCacheTime() < $NOW) {
+		foreach ($locationData as $row) {
+			// Check cache age
+			if ($backgroundPrefetch || $row['lastcalendarupdate'] + $this->getCacheTime() < $NOW) {
+				// Too old - add to remoteIds, which is the list of rooms/locations we will eventually query
 				$remoteIds[$row['locationid']] = $row['serverlocationid'];
 			}
 			// Always add to return value - if updating fails, we better use the stale data than nothing
 			$returnValue[$row['locationid']] = json_decode($row['calendar'], true);
 		}
+		if (!$backgroundPrefetch) {
+			// Update lastuse timestamp
+			Database::simpleQuery("UPDATE locationinfo_locationconfig SET lastuse = UNIX_TIMESTAMP()
+                                   WHERE locationinfo_locationconfig.serverid = :sid AND serverlocationid IN (:slocs)",
+				['sid' => $this->serverId, 'slocs' => array_column($locationData, 'serverlocationid')]);
+		}
 		// No need for additional round trips to backend
 		if (empty($remoteIds)) {
 			return $returnValue;
 		}
-		// Mark requested locations as used
-		Database::exec("UPDATE locationinfo_locationconfig SET lastuse = :now WHERE locationid IN (:locations)",
-			['locations' => $requestedLocationIds, 'now' => $NOW]);
-		// Check if we should refresh other rooms recently requested by front ends
-		$extraLocs = self::TRY_NUM_LOCATIONS - count($remoteIds);
-		if ($this->getRefreshTime() > $this->getCacheTime() && $extraLocs > 0) {
-			$dbquery4 = Database::simpleQuery("SELECT locationid, serverlocationid FROM locationinfo_locationconfig
-					WHERE serverid = :serverid AND serverlocationid NOT IN (:skiplist)
-					AND lastcalendarupdate < :minage AND lastuse > :lastuse
-					LIMIT $extraLocs", array(
+		// Update the last update timestamp of the ones we are working on, so they won't be queried in parallel
+		// if another request comes in while we're in fetchSchedulesInternal. Currently done without locking the
+		// table. I think it's unlikely enough that we still get a race during those three queries here, and even
+		// if: nothing bad will happen...
+		// Also update lastuse if not ignoring cache (as this means we're opportunistically fetching
+		Database::exec("UPDATE locationinfo_locationconfig
+					SET lastcalendarupdate = :time
+					WHERE lastcalendarupdate < :time AND serverid = :serverid AND serverlocationid IN (:slocs)", [
+						'time' => $NOW - ($this->getCacheTime() - 300), // Protect for 5 minutes - this also applies for failed remote queries
 						'serverid' => $this->serverId,
-						'skiplist' => array_values($remoteIds),
-						'lastuse' => $NOW - $this->getRefreshTime(),
-						'minage' => $NOW - $this->getCacheTime(),
-			));
-			foreach ($dbquery4 as $row) {
-				$remoteIds[$row['locationid']] = $row['serverlocationid'];
-			}
-		}
-		if ($this->getCacheTime() > 0) {
-			// Update the last update timestamp of the ones we are working on, so they won't be queried in parallel
-			// if another request comes in while we're in fetchSchedulesInternal. Currently done without locking the
-			// table. I think it's unlikely enough that we still get a race during those three queries here, and even
-			// if, nothing bad will happen...
-			Database::exec("UPDATE locationinfo_locationconfig SET lastcalendarupdate = :time
-						WHERE lastcalendarupdate < :time AND serverid = :serverid AND serverlocationid IN (:slocs)", [
-							'time' => $NOW - ($this->getCacheTime() - 60), // Protect for one minute max.
-							'serverid' => $this->serverId,
-							'slocs' => array_values($remoteIds),
-			]);
-		}
+						'slocs' => array_values($remoteIds),
+		]);
 		$backendResponse = $this->fetchSchedulesInternal(array_unique($remoteIds), $deadline);
 
 		// Fetching might have taken a while, get current time again
@@ -261,11 +258,11 @@ abstract class CourseBackend
 		}
 		unset($calendar);
 		// Add rooms that were requested to the final return value
-		foreach ($remoteIds as $location => $serverRoomId) {
+		foreach ($remoteIds as $locationId => $serverRoomId) {
 			if (isset($backendResponse[$serverRoomId]) && is_array($backendResponse[$serverRoomId])
-					&& in_array($location, $requestedLocationIds)) {
+					&& in_array($locationId, $requestedLocationIds)) {
 				// Only add if we can map it back to our location id AND it was not an unsolicited coalesced refresh
-				$returnValue[$location] = $backendResponse[$serverRoomId];
+				$returnValue[$locationId] = $backendResponse[$serverRoomId];
 			}
 		}
 
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php
index 44879caa..1d42f0bc 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php
@@ -56,12 +56,12 @@ class CourseBackend_Davinci extends CourseBackend
 
 	public function getCacheTime(): int
 	{
-		return 30 * 60;
+		return 40 * 60;
 	}
 
 	public function getRefreshTime(): int
 	{
-		return 0;
+		return 20 * 60;
 	}
 
 	/**
@@ -120,7 +120,7 @@ class CourseBackend_Davinci extends CourseBackend
 			$lessons = $this->getArrayPath($return, '/Lessons/Lesson');
 			if ($lessons === false) {
 				$this->addError("Cannot find /Lessons/Lesson in XML", false);
-				continue;
+				$lessons = []; // So we return an empty table and cache it
 			}
 			$timetable = [];
 			foreach ($lessons as $lesson) {
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php
index 8ca72e92..7ec5e1aa 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php
@@ -64,7 +64,7 @@ class CourseBackend_Dummy extends CourseBackend
 	}
 
 	/**
-	 * @return int desired caching time of results, in seconds. 0 = no caching
+	 * @return int desired caching time of results, in seconds. 0 = no caching.
 	 */
 	public function getCacheTime(): int
 	{
@@ -72,8 +72,9 @@ class CourseBackend_Dummy extends CourseBackend
 	}
 
 	/**
-	 * @return int age after which timetables are no longer refreshed should be
-	 * greater then CacheTime
+	 * @return int if cached calendar is at least this old, and there is still
+	 * a frontend actively using this calendar, try to refresh the calendar
+	 * in the background. 0 = disable.
 	 */
 	public function getRefreshTime(): int
 	{
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_exchange.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_exchange.inc.php
index b4a35139..8c172a00 100755
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_exchange.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_exchange.inc.php
@@ -136,7 +136,7 @@ class CourseBackend_Exchange extends CourseBackend
 	 */
 	public function getCacheTime(): int
 	{
-		return 15 * 60;
+		return 40 * 60;
 	}
 
 	/**
@@ -145,7 +145,7 @@ class CourseBackend_Exchange extends CourseBackend
 	 */
 	public function getRefreshTime(): int
 	{
-		return 30 * 60;
+		return 15 * 60;
 	}
 
 	/**
@@ -166,7 +166,7 @@ class CourseBackend_Exchange extends CourseBackend
 				$timeout = $deadline - time();
 				if ($timeout <= 1)
 					break;
-				$client->setCurlOption(CURLOPT_TIMEOUT, min($timeout, 60));
+				$client->setCurlOption(CURLOPT_TIMEOUT, min($timeout, 20));
 			}
 			try {
 				$items = $this->findEventsForRoom($client, $startDate, $endDate, $roomId);
@@ -176,6 +176,7 @@ class CourseBackend_Exchange extends CourseBackend
 			}
 
 			// Iterate over the events that were found, printing some data for each.
+			$schedules[$roomId] = [];
 			foreach ($items as $item) {
 				try {
 					$start = new DateTime($item->Start);
@@ -238,7 +239,7 @@ class CourseBackend_Exchange extends CourseBackend
 		return $items;
 	}
 
-	public function getClient(): Client
+	private function getClient(): Client
 	{
 		$client = new Client($this->serverAddress, $this->username, $this->password, $this->clientVersion);
 		$client->setTimezone($this->timezone);
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php
index 77077074..1ed805b2 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php
@@ -28,7 +28,7 @@ class CourseBackend_HisInOne extends ICalCourseBackend
 	public function mangleProperty(string $prop, $value)
 	{
 		if ($prop === 'baseUrl') {
-			// Update form SOAP to iCal url
+			// Update from SOAP to iCal url
 			if (preg_match(',^(http.*?)/qisserver,', $value, $out)) {
 				$value = $out[1] . '/qisserver/pages/cm/exa/timetable/roomScheduleCalendarExport.faces?roomId=%ID%';
 			} elseif (preg_match(',(.*[/=])\d*$', $value, $out)) {
@@ -55,7 +55,7 @@ class CourseBackend_HisInOne extends ICalCourseBackend
 	{
 		if (!$this->isOK())
 			return false;
-		// Unfortunately HisInOne returns an internal server error if you pass an invalid roomId.
+		// Unfortunately, HisInOne returns an internal server error if you pass an invalid roomId.
 		// So we just try a bunch and see if anything works. Even if this fails, using
 		// the backend should work, given the URL is actually correct.
 		foreach ([60, 100, 5, 10, 11, 12, 13, 14, 15, 50, 110, 200, 210, 250, 300, 333, 500, 1000, 2000] as $roomId) {
@@ -78,16 +78,13 @@ class CourseBackend_HisInOne extends ICalCourseBackend
 		foreach ($events as $event) {
 			if (isset($event->url) && preg_match('/[&?]periodId=([0-9]+)(&|$)/', $event->url, $out)) {
 				// Collect periodIds from URLs to find more lectures. While the HisInOne help says that the periodId
-				// parameter can simply be removed, in practice that means some lectures that take place even on the
-				// current day or week might not be returned in the resulting ical file, unless you pass the proper
-				// current periodId. Since there is no API or similar that reliably returns the current periodId,
-				// we just collect all the periodIds we find in the original ical file, and re-query with each of
-				// those appended to the URL, and merge the final result.
-				// Also, some courses seem to be assigned to past semesters, i.e. as of writing we have July 2025,
-				// but there is a lecture that won't be returned in the iCal data unless we make a request for
-				// the periodId corresponding to summer term 2024 (!)
+				// parameter can simply be removed, this is not true. In practice, some lectures that take place even
+				// on the current day or week might not be returned in the resulting ical file, unless you the proper
+				// current periodId. BUT WAIT THERE'S MORE: Even with the current periodId, some lectures don't get
+				// returned, so we start brute-forcing semi-random periodIds, while at the same time discovering
+				// new ones from the returned calendars.
 				$periodId = (int)$out[1];
-				for ($i = min($periodId - 3, 1); $i <= $periodId + 3; $i++) {
+				for ($i = max($periodId - 2, 1); $i <= $periodId + 2; $i++) {
 					$periodIds[$i] = $i;
 				}
 			}
@@ -98,26 +95,30 @@ class CourseBackend_HisInOne extends ICalCourseBackend
 		// Now re-query with all the periodIds we found
 		$dupCheck = [];
 		while (($periodId = array_pop($periodIds)) !== null) {
+			if (count($dupCheck) > 35)
+				break;
 			if (isset($dupCheck[$periodId]))
 				continue;
-			if ($deadline !== null && $deadline - time() <= 1)
+			if ($deadline !== null && $deadline - time() <= 1) {
+				error_log("Timeout exceeded (" . count($finalEvents) . " events & " . count($dupCheck) . " periodIds) for $roomId");
 				break;
+			}
 			$dupCheck[$periodId] = true;
-			$events = parent::downloadIcal($roomId, $deadline, '&periodId=' . $periodId);
+			$events = parent::downloadIcal($roomId, $deadline, '&periodId=' . $periodId, $limitRange);
 			if (empty($events))
 				continue;
 			foreach ($events as $event) {
-				$finalEvents[$this->hash($event)] = $event;
+				if (strtotime($event->dtstart) <= $dtEnd && strtotime($event->dtend) >= $dtStart) {
+					$finalEvents[$this->hash($event)] = $event;
+				}
 				// Collect more periodIds
 				if (isset($event->url) && preg_match('/[&?]periodId=([0-9]+)(&|$)/', $event->url, $out)) {
-					$periodId = (int)$out[1];
-					for ($i = min($periodId - 3, 1); $i <= $periodId + 3; $i++) {
+					$newPeriodId = (int)$out[1];
+					for ($i = max($newPeriodId - 2, 1); $i <= $newPeriodId + 2; $i++) {
 						$periodIds[$i] = $i;
 					}
 				}
 			}
-			if (count($dupCheck) > 5)
-				break;
 		}
 		return array_values($finalEvents);
 	}
@@ -129,12 +130,12 @@ class CourseBackend_HisInOne extends ICalCourseBackend
 
 	public function getCacheTime(): int
 	{
-		return 30 * 60;
+		return 40 * 60;
 	}
 
 	public function getRefreshTime(): int
 	{
-		return 60 * 60;
+		return 20 * 60;
 	}
 
 
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_ical.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_ical.inc.php
index dbe53326..740bbc8b 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_ical.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_ical.inc.php
@@ -44,12 +44,12 @@ class CourseBackend_ICal extends ICalCourseBackend
 
 	public function getCacheTime(): int
 	{
-		return 30 * 60;
+		return 40 * 60;
 	}
 
 	public function getRefreshTime(): int
 	{
-		return 60 * 60;
+		return 20 * 60;
 	}
 
 
diff --git a/modules-available/locationinfo/inc/icalcoursebackend.inc.php b/modules-available/locationinfo/inc/icalcoursebackend.inc.php
index c371fba2..c9113ec6 100644
--- a/modules-available/locationinfo/inc/icalcoursebackend.inc.php
+++ b/modules-available/locationinfo/inc/icalcoursebackend.inc.php
@@ -71,7 +71,7 @@ abstract class ICalCourseBackend extends CourseBackend
 			CURLOPT_SSL_VERIFYHOST => $this->verifyHostname ? 2 : 0,
 			CURLOPT_SSL_VERIFYPEER => $this->verifyCert ? 1 : 0,
 			CURLOPT_URL => str_replace('%ID%', $roomId, $this->location . ($appendToUrl ?? '')),
-			CURLOPT_TIMEOUT => min(60, $timeout),
+			CURLOPT_TIMEOUT => min(20, $timeout),
 			CURLOPT_CONNECTTIMEOUT => 4,
 		];
 		if ($this->authMethod !== 'NONE' && defined('CURLAUTH_' . $this->authMethod)) {
@@ -112,6 +112,7 @@ abstract class ICalCourseBackend extends CourseBackend
 				$this->addError("Downloading ical for $roomId failed", false);
 				continue;
 			}
+			$tTables[$roomId] = [];
 			foreach ($data as $event) {
 				$tTables[$roomId][] = array(
 					'title' => $this->toTitle($event),
diff --git a/modules-available/locationinfo/inc/locationinfo.inc.php b/modules-available/locationinfo/inc/locationinfo.inc.php
index 1775dce5..511ed3ea 100644
--- a/modules-available/locationinfo/inc/locationinfo.inc.php
+++ b/modules-available/locationinfo/inc/locationinfo.inc.php
@@ -359,7 +359,7 @@ class LocationInfo
 	}
 
 	/**
-	 * Gets the calendar of the given ids.
+	 * Gets the calendar of the given location IDs.
 	 *
 	 * @param int[] $idList list with the location ids.
 	 * @param ?int $deadline null = no timeout, otherwise a unix timestamp until which we expect function to return
@@ -370,10 +370,9 @@ class LocationInfo
 		if (empty($idList))
 			return [];
 
-		$resultArray = array();
-
 		if ($deadline !== null && $deadline <= time()) {
 			// Deadline in the past, use cached data exclusively
+			$resultArray = [];
 			$res = Database::simpleQuery("SELECT locationid, calendar FROM locationinfo_locationconfig
 					WHERE Length(calendar) > 10
 					  AND lastcalendarupdate > UNIX_TIMESTAMP() - 86400*3
@@ -392,46 +391,84 @@ class LocationInfo
 		$query = "SELECT l.locationid, l.serverid, l.serverlocationid, s.servertype, s.credentials
 				FROM `locationinfo_locationconfig` AS l
 				INNER JOIN locationinfo_coursebackend AS s ON (s.serverid = l.serverid)
-				WHERE l.locationid IN (:idlist)
-				ORDER BY s.servertype ASC";
-		$dbquery = Database::simpleQuery($query, array('idlist' => array_values($idList)));
+				WHERE l.locationid IN (:idlist)";
+		$rows = Database::queryAll($query, array('idlist' => array_values($idList)));
+		return self::getCalendarCommon($rows, $deadline);
+	}
 
-		$serverList = array();
-		foreach ($dbquery as $dbresult) {
-			if (!isset($serverList[$dbresult['serverid']])) {
-				$serverList[$dbresult['serverid']] = array(
-					'credentials' => (array)json_decode($dbresult['credentials'], true),
-					'type' => $dbresult['servertype'],
-					'idlist' => array()
-				);
+	/**
+	 * Fetch all calendars from backend that seem to be actively in use by a running frontend panel,
+	 * and are about to expire. This should reduce load times next time the panel requests an update,
+	 * as it ensures the data can be served from cache.
+	 */
+	public static function refreshStaleButActiveCalendars()
+	{
+		$NOW = time();
+		// Get all calendars where 'lastuse' was in past ~6 minutes
+		$rows = Database::queryAll("SELECT l.locationid, l.lastcalendarupdate, l.serverid, l.serverlocationid,
+       			s.servertype, s.credentials
+				FROM `locationinfo_locationconfig` AS l
+				INNER JOIN locationinfo_coursebackend AS s ON (s.serverid = l.serverid)
+				WHERE l.lastuse > :cutoff AND l.lastuse > l.lastcalendarupdate", ['cutoff' => $NOW - 400]);
+		foreach (array_keys($rows) as $i) {
+			$row = $rows[$i];
+			$serverInstance = CourseBackend::getInstance($row['servertype']);
+			if ($serverInstance === false)
+				continue;
+			$secsSinceLastFetch = $NOW - $row['lastcalendarupdate'];
+			// Must hold:
+			// Last update was between max cache time and opportunistic refresh time
+			// Also refresh time has to be at least 10 minutes
+			if ($serverInstance->getRefreshTime() < 600
+					|| $secsSinceLastFetch > $serverInstance->getCacheTime()
+					|| $secsSinceLastFetch < $serverInstance->getRefreshTime()) {
+				// Nope
+				unset($rows[$i]);
 			}
-			$serverList[$dbresult['serverid']]['idlist'][] = $dbresult['locationid'];
 		}
+		self::getCalendarCommon($rows, -1);
+	}
 
+	/**
+	 * @param ?int $deadline accepts special value -1 to ignore cache and always fetch
+	 */
+	private static function getCalendarCommon(array $wantedCalendars, ?int $deadline): array
+	{
+		$serverList = [];
+		foreach ($wantedCalendars as $cal) {
+			if (!isset($serverList[$cal['serverid']])) {
+				$serverList[$cal['serverid']] = [
+					'credentials' => (array)json_decode($cal['credentials'], true),
+					'type' => $cal['servertype'],
+					'idlist' => []
+				];
+			}
+			$serverList[$cal['serverid']]['idlist'][] = $cal['locationid'];
+		}
+
+		$resultArray = [];
 		foreach ($serverList as $serverid => $server) {
 			$serverInstance = CourseBackend::getInstance($server['type']);
 			if ($serverInstance === false) {
-				EventLog::warning('Cannot fetch schedule for location (' . implode(', ', $server['idlist']) . ')'
+				EventLog::warning('Cannot fetch schedule (' . implode(', ', $server['idlist']) . ')'
 					. ': Backend type ' . $server['type'] . ' unknown. Disabling location.');
 				Database::exec("UPDATE locationinfo_locationconfig SET serverid = NULL WHERE locationid IN (:lid)",
-					array('lid' => $server['idlist']));
+					['lid' => $server['idlist']]);
 				continue;
 			}
-			$credentialsOk = $serverInstance->setCredentials($serverid, $server['credentials']);
-
-			if ($credentialsOk) {
+			if ($serverInstance->setCredentials($serverid, $server['credentials'])) {
 				$calendarFromBackend = $serverInstance->fetchSchedule($server['idlist'], $deadline);
 			} else {
-				$calendarFromBackend = array();
+				$calendarFromBackend = [];
 			}
 
 			LocationInfo::setServerError($serverid, $serverInstance->getErrors());
 
-			foreach ($calendarFromBackend as $key => $value) {
-				$resultArray[] = array(
-					'id' => (int)$key,
-					'calendar' => $value,
-				);
+			foreach ($calendarFromBackend as $locationId => $calendar) {
+				$resultArray[] = [
+					'id' => (int)$locationId,
+					'calendar' => $calendar,
+				];
 			}
 		}
 		return $resultArray;
@@ -443,7 +480,7 @@ class LocationInfo
 	 * we request the function to return, trying to update as many stale calendars as possible during that time,
 	 * returning cached values for all that weren't updated. Passing a timestamp in the past (e.g. 0) ensures
 	 * that all calendars will be served from cache, regardless of age.
-	 * @param ?int $deadline null = no timeout, otherwise a unix timestamp until which we expect function to return
+	 * @param ?int $deadline null = no timeout, otherwise a unix timestamp until which we expect to return
 	 */
 	public static function getAllCalendars(?int $deadline = null): array
 	{