[locationinfo] CourseBackends: Lots of bug fixes, missing error checks, improvements:

- Add more checks for returned data structures from backend, like keys in arrays
- Better error messages if something goes wrong, not just "server sent wrong xml"
- Make checkConnection() of davinci and hisinone not require a valid room id, which
  we don't have in general
- hisinone: Parse data structure just once for every room
- Request coalescing: Only try so if getRefreshTime() > getCacheTime()
- Move toArray() to base class instead of having two copies
- Sanitize variable naming conventions

4 files changed, 238 insertions, 263 deletions

diff --git a/modules-available/locationinfo/inc/coursebackend.inc.php b/modules-available/locationinfo/inc/coursebackend.inc.php
index 11d833e6..7dc50549 100644
--- a/modules-available/locationinfo/inc/coursebackend.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend.inc.php
@@ -12,17 +12,24 @@ abstract class CourseBackend
 
 	/**
 	 * @var array list of known backends
-	 * @var boolean  true if there was an error
-	 * @var string with the error message
-	 * @var int as internal serverID
-	 * @var string url of the service
 	 */
 	private static $backendTypes = false;
-	public $error;
-	public $errormsg;
-	public $serverID;
-	public $location;
-	const nrOtherRooms = 5;
+	/**
+	 * @var boolean|string false = no error, error message otherwise
+	 */
+	protected $error;
+	/**
+	 * @var int as internal serverId
+	 */
+	protected $serverId;
+	/**
+	 * @var string url of the service
+	 */
+	protected $location;
+	/**
+	 * @const int max number of additional locations to fetch (for backends that benefit from request coalesc.)
+	 */
+	const MAX_ADDIDIONAL_LOCATIONS = 5;
 
 	/**
 	 * CourseBackend constructor.
@@ -31,7 +38,6 @@ abstract class CourseBackend
 	{
 		$this->location = "";
 		$this->error = false;
-		$this->errormsg = "";
 	}
 
 	/**
@@ -68,7 +74,7 @@ abstract class CourseBackend
 	 * Get fresh instance of ConfigModule subclass for given module type.
 	 *
 	 * @param string $moduleType name of module type
-	 * @return \ConfigModule module instance
+	 * @return \CourseBackend module instance
 	 */
 	public static function getInstance($moduleType)
 	{
@@ -106,10 +112,10 @@ abstract class CourseBackend
 	 *
 	 * @param array $data with the credentials
 	 * @param string $url address of the server
-	 * @param int $serverID ID of the server
+	 * @param int $serverId ID of the server
 	 * @returns bool if the credentials were in the correct format
 	 */
-	public abstract function setCredentials($data, $url, $serverID);
+	public abstract function setCredentials($data, $url, $serverId);
 
 	/**
 	 * @return int desired caching time of results, in seconds. 0 = no caching
@@ -125,7 +131,7 @@ abstract class CourseBackend
 	/**
 	 * Internal version of fetch, to be overridden by subclasses.
 	 *
-	 * @param $roomIds array with local ID as key and serverID as value
+	 * @param $roomIds array with local ID as key and serverId as value
 	 * @return array a recursive array that uses the roomID as key
 	 * and has the schedule array as value. A shedule array contains an array in this format:
 	 * ["start"=>'JJJJ-MM-DD HH:MM:SS',"end"=>'JJJJ-MM-DD HH:MM:SS',"title"=>string]
@@ -138,80 +144,75 @@ abstract class CourseBackend
 	 * @param array $roomId array of room ID to fetch
 	 * @return array|bool array containing the timetables as value and roomid as key as result, or false on error
 	 */
-	public final function fetchSchedule($roomIDs)
+	public final function fetchSchedule($requestedLocationIds)
 	{
-		if (empty($roomIDs)) {
-			$this->error = true;
-			$this->errormsg = 'No roomid was given to fetch Shedule';
+		if (!is_array($requestedLocationIds)) {
+			$this->error = 'No array of roomids was given to fetchSchedule';
 			return false;
 		}
-		$sqlr = implode(",", $roomIDs);
-		$sqlr = '(' . $sqlr . ')';
-		$q = "SELECT locationid, calendar, serverroomid, lastcalendarupdate FROM location_info WHERE locationid IN " . $sqlr;
-		$dbquery1 = Database::simpleQuery($q);
-		$result = [];
-		$sRoomIDs = [];
-		$newResult = [];
-		foreach ($dbquery1->fetchAll(PDO::FETCH_ASSOC) as $row) {
-			$sRoomID = $row['serverroomid'];
-			$lastUpdate = $row['lastcalendarupdate'];
-			$calendar = $row['calendar'];
+		if (empty($requestedLocationIds))
+			return array();
+		$NOW = time();
+		$dbquery1 = Database::simpleQuery("SELECT locationid, calendar, serverroomid, lastcalendarupdate
+				FROM location_info WHERE locationid IN (:locations)",
+				array('locations' => array_values($requestedLocationIds)));
+		$returnValue = [];
+		$remoteIds = [];
+		while ($row = $dbquery1->fetch(PDO::FETCH_ASSOC)) {
 			//Check if in cache if lastUpdate is null then it is interpreted as 1970
-			if ($lastUpdate > strtotime("-" . $this->getCacheTime() . "seconds")) {
-				$result[$row['locationid']] = json_decode($calendar);
+			if ($row['lastcalendarupdate'] + $this->getCacheTime() > $NOW) {
+				$returnValue[$row['locationid']] = json_decode($row['calendar']);
 			} else {
-				$sRoomIDs[$row['locationid']] = $sRoomID;
+				$remoteIds[$row['locationid']] = $row['serverroomid'];
 			}
 
 		}
-		//Check if we should refresh other rooms recently requested by front ends
-		if ($this->getCacheTime() > 0) {
-			$i = 0; //number of rooms getting refreshed
-			$dbquery4 = Database::simpleQuery("SELECT locationid ,serverroomid, lastcalendarupdate FROM location_info WHERE serverid= :id", array('id' => $this->serverID));
-			foreach ($dbquery4->fetchAll(PDO::FETCH_COLUMN) as $row) {
-				if (isset($row['lastcalendarupdate'])) {
-					$lastUpdate = $row['lastcalendarupdate'];
-					if ($lastUpdate < strtotime("-" . $this->getRefreshTime() . "seconds")
-							&& $lastUpdate > strtotime("-" . $this->getCacheTime() . "seconds"
-							&& $i < self::nrOtherRooms)) {
-						$sRoomIDs[$row['locationid']] = $row['serverroomid'];
-						$i = $i + 1;
-					}
-				}
-			}
+		// No need for additional round trips to backend
+		if (empty($remoteIds)) {
+			return $returnValue;
 		}
-		//This is true if there is no need to check the HisInOne Server
-		if (empty($sRoomIDs)) {
-			return $result;
+		// Check if we should refresh other rooms recently requested by front ends
+		if ($this->getRefreshTime() > $this->getCacheTime()) {
+			$dbquery4 = Database::simpleQuery("SELECT locationid, serverroomid FROM location_info
+					WHERE serverid = :serverid AND serverroomid NOT IN (:skiplist)
+					AND lastcalendarupdate BETWEEN :lowerage AND :upperage
+					LIMIT " . self::MAX_ADDIDIONAL_LOCATIONS, array(
+						'serverid' => $this->serverId,
+						'skiplist' => array_values($remoteIds),
+						'lowerage' => $NOW - $this->getRefreshTime(),
+						'upperage' => $NOW - $this->getCacheTime(),
+			));
+			while ($row = $dbquery4->fetch(PDO::FETCH_ASSOC)) {
+				$remoteIds[$row['locationid']] = $row['serverroomid'];
+			}
 		}
-		$results = $this->fetchSchedulesInternal($sRoomIDs);
-		if ($results === false) {
+		$backendResponse = $this->fetchSchedulesInternal($remoteIds);
+		if ($backendResponse === false) {
 			return false;
 		}
 
-		foreach ($sRoomIDs as $location => $serverRoom) {
-			$newResult[$location] = $results[$serverRoom];
-		}
-
 		if ($this->getCacheTime() > 0) {
-			foreach ($newResult as $key => $value) {
-				$value = json_encode($value);
-				$now = strtotime('Now');
+			// Caching requested by backend, write to DB
+			foreach ($backendResponse as $serverRoomId => $calendar) {
+				$value = json_encode($calendar);
 				Database::simpleQuery("UPDATE location_info SET calendar = :ttable, lastcalendarupdate = :now
-					WHERE locationid = :id ", array(
-					'id' => $key,
+					WHERE serverid = :serverid AND serverroomid = :serverroomid", array(
+					'serverid' => $this->serverId,
+					'serverroomid' => $serverRoomId,
 					'ttable' => $value,
-					'now' => $now
+					'now' => $NOW
 				));
 			}
 		}
-		//get all schedules that are wanted from roomIDs
-		foreach ($roomIDs as $id) {
-			if (isset($newResult[$id])) {
-				$result[$id] = $newResult[$id];
+		// Add rooms that were requested to the final return value
+		foreach ($remoteIds as $location => $serverRoomId) {
+			if (isset($backendResponse[$serverRoomId]) && in_array($location, $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];
 			}
 		}
-		return $result;
+
+		return $returnValue;
 	}
 
 	/**
@@ -219,10 +220,7 @@ abstract class CourseBackend
 	 */
 	public final function getError()
 	{
-		if ($this->error) {
-			return $this->errormsg;
-		}
-		return false;
+		return $this->error;
 	}
 
 	/**
@@ -232,7 +230,7 @@ abstract class CourseBackend
 	 * and leaf nodes. The result is always an array on success, or
 	 * false if not found.
 	 */
-	function getAttributes($array, $path)
+	protected function getAttributes($array, $path)
 	{
 		if (!is_array($path)) {
 			// Convert 'path/syntax/foo/wanteditem' to array for further processing and recursive calls
@@ -280,4 +278,22 @@ abstract class CourseBackend
 		// Unique non-leaf node - simple recursion
 		return $this->getAttributes($array[$element], $path);
 	}
+
+	/**
+	 * @param string $response xml document to convert
+	 * @return bool|array array representation of the xml if possible, false otherwise
+	 */
+	protected function toArray($response)
+	{
+		$cleanresponse = preg_replace('/(<\/?)(\w+):([^>]*>)/', '$1$2$3', $response);
+		try {
+			$xml = new SimpleXMLElement($cleanresponse);
+		} catch (Exception $e) {
+			$this->error = 'Could not parse reply as XML, got ' . get_class($e) . ': ' . $e->getMessage();
+			return false;
+		}
+		$array = json_decode(json_encode((array)$xml), true);
+		return $array;
+	}
+
 }
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php
index 11882a1e..45fd51f5 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php
@@ -1,37 +1,36 @@
 <?php
 
-class Coursebackend_Davinci extends CourseBackend
+class CourseBackend_Davinci extends CourseBackend
 {
 
-
-	public function setCredentials($data, $location, $serverID)
+	public function setCredentials($data, $location, $serverId)
 	{
-		if ($location == "") {
-			$this->error = true;
-			$this->errormsg = "No url is given";
-			return !$this->error;
+		if (empty($location)) {
+			$this->error = "No url is given";
+			return false;
 		}
 		$this->location = $location . "/DAVINCIIS.dll?";
-		$this->serverID = $serverID;
+		$this->serverId = $serverId;
 		//Davinci doesn't have credentials
 		return true;
 	}
 
 	public function checkConnection()
 	{
-		if ($this->location != "") {
-			$this->fetchSchedulesInternal(['B206']);
-			return !$this->error;
+		if (empty($this->location)) {
+			$this->error = "Credentials are not set";
+		} else {
+			$data = $this->fetchRoomRaw('someroomid123');
+			if (strpos($data, 'DAVINCI SERVER') === false) {
+				$this->error = "This doesn't seem to be a DAVINCI server";
+			}
 		}
-		$this->error = true;
-		$this->errormsg = "Credentials are not set";
-		return !$this->error;
+		return $this->error === false;
 	}
 
 	public function getCredentials()
 	{
-		$return = array();
-		return $return;
+		return array();
 	}
 
 	public function getDisplayName()
@@ -50,32 +49,15 @@ class Coursebackend_Davinci extends CourseBackend
 	}
 
 	/**
-	 * @param $response xml document
-	 * @return bool|array array representation of the xml if possible
-	 */
-	private function toArray($response)
-	{
-		try {
-			$cleanresponse = preg_replace('/(<\/?)(\w+):([^>]*>)/', "$1$2$3", $response);
-			$xml = new SimpleXMLElement($cleanresponse);
-			$array = json_decode(json_encode((array)$xml), true);
-		} catch (Exception $exception) {
-			$this->error = true;
-			$this->errormsg = "url did not answer with a xml, maybe the url is wrong or the room is wrong";
-			$array = false;
-		}
-		return $array;
-	}
-
-	/**
 	 * @param $roomId string name of the room
 	 * @return array|bool if successful the arrayrepresentation of the timetable
 	 */
-	private function fetchArray($roomId)
+	private function fetchRoomRaw($roomId)
 	{
 		$startDate = new DateTime('today 0:00');
 		$endDate = new DateTime('+7 days 0:00');
-		$url = $this->location . "content=xml&type=room&name=" . $roomId . "&startdate=" . $startDate->format('d.m.Y') . "&enddate=" . $endDate->format('d.m.Y');
+		$url = $this->location . "content=xml&type=room&name=" . urlencode($roomId)
+			. "&startdate=" . $startDate->format('d.m.Y') . "&enddate=" . $endDate->format('d.m.Y');
 		$ch = curl_init();
 		$options = array(
 			CURLOPT_RETURNTRANSFER => true,
@@ -88,53 +70,55 @@ class Coursebackend_Davinci extends CourseBackend
 		curl_setopt_array($ch, $options);
 		$output = curl_exec($ch);
 		if ($output === false) {
-			$this->error = true;
-			$this->errormsg = 'Curl error: ' . curl_error($ch) . $url;
+			$this->error = 'Curl error: ' . curl_error($ch);
 			return false;
 		} else {
 			$this->error = false;
-			$this->errormsg = "";
 			///Operation completed successfully
 		}
 		curl_close($ch);
-		error_log($output);
-		return $this->toArray($output);
+		return $output;
 
 	}
 
-	public function fetchSchedulesInternal($roomIds)
+	public function fetchSchedulesInternal($requestedRoomIds)
 	{
 		$schedules = [];
-		foreach ($roomIds as $sroomId) {
-			$return = $this->fetchArray($sroomId);
+		foreach ($requestedRoomIds as $roomId) {
+			$return = $this->fetchRoomRaw($roomId);
 			if ($return === false) {
-				return false;
+				continue;
 			}
-			$lessons = $this->getAttributes($return, 'Lessons/Lesson');
-			if (!$lessons) {
-				$this->error = true;
-				$this->errormsg = "url send a xml in a wrong format";
-				return false;
+			$return = $this->toArray($return);
+			if ($return === false) {
+				continue;
+			}
+			$lessons = $this->getAttributes($return, '/Lessons/Lesson');
+			if ($lessons === false) {
+				$this->error = "Cannot find /Lessons/Lesson in XML";
+				continue;
 			}
 			$timetable = [];
 			foreach ($lessons as $lesson) {
+				if (!isset($lesson['Date']) || !isset($lesson['Start']) || !isset($lesson['Finish'])) {
+					$this->error = 'Lesson is missing Date, Start or Finish';
+					continue;
+				}
 				$date = $lesson['Date'];
 				$date = substr($date, 0, 4) . '-' . substr($date, 4, 2) . '-' . substr($date, 6, 2);
 				$start = $lesson['Start'];
 				$start = substr($start, 0, 2) . ':' . substr($start, 2, 2);
 				$end = $lesson['Finish'];
 				$end = substr($end, 0, 2) . ':' . substr($end, 2, 2);
-				$subject = $lesson['Subject'];
-				$json = array(
+				$subject = isset($lesson['Subject']) ? $lesson['Subject'] : '???';
+				$timetable[] = array(
 					'title' => $subject,
 					'start' => $date . " " . $start . ':00',
 					'end' => $date . " " . $end . ':00'
 				);
-				array_push($timetable, $json);
 			}
-			$schedules[$sroomId] = $timetable;
+			$schedules[$roomId] = $timetable;
 		}
 		return $schedules;
 	}
 }
-
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php
index 484a5286..5bceac3e 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php
@@ -1,6 +1,6 @@
 <?php
 
-class Coursebackend_Dummy extends CourseBackend
+class CourseBackend_Dummy extends CourseBackend
 {
 	private $pw;
 
@@ -10,10 +10,10 @@ class Coursebackend_Dummy extends CourseBackend
 	 *
 	 * @param array $data with the credentials
 	 * @param string $url address of the server
-	 * @param int $serverID ID of the server
+	 * @param int $serverId ID of the server
 	 * @returns bool if the credentials were in the correct format
 	 */
-	public function setCredentials($json, $location, $serverID)
+	public function setCredentials($json, $location, $serverId)
 	{
 		$x = $json;
 		$this->pw = $x['password'];
@@ -22,8 +22,7 @@ class Coursebackend_Dummy extends CourseBackend
 			$this->error = false;
 			return true;
 		} else {
-			$this->errormsg = "USE mfg as password!";
-			$this->error = true;
+			$this->error = "USE mfg as password!";
 			return false;
 		}
 	}
@@ -37,8 +36,7 @@ class Coursebackend_Dummy extends CourseBackend
 			$this->error = false;
 			return true;
 		} else {
-			$this->errormsg = "USE mfg as password!";
-			$this->error = true;
+			$this->error = "USE mfg as password!";
 			return false;
 		}
 	}
@@ -88,7 +86,7 @@ class Coursebackend_Dummy extends CourseBackend
 	/**
 	 * Internal version of fetch, to be overridden by subclasses.
 	 *
-	 * @param $roomIds array with local ID as key and serverID as value
+	 * @param $roomIds array with local ID as key and serverId as value
 	 * @return array a recursive array that uses the roomID as key
 	 * and has the schedule array as value. A shedule array contains an array in this format:
 	 * ["start"=>'JJJJ-MM-DD HH:MM:SS',"end"=>'JJJJ-MM-DD HH:MM:SS',"title"=>string]
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php
index 0e7c5328..9e7c9db0 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php
@@ -7,7 +7,7 @@ class CourseBackend_HisInOne extends CourseBackend
 	private $open;
 
 
-	public function setCredentials($data, $url, $serverID)
+	public function setCredentials($data, $url, $serverId)
 	{
 		if (array_key_exists('password', $data) && array_key_exists('username', $data) && array_key_exists('role', $data) && isset($data['open'])) {
 			$this->error = false;
@@ -15,19 +15,17 @@ class CourseBackend_HisInOne extends CourseBackend
 			$this->username = $data['username'] . "\t" . $data['role'];
 			$this->open = $data['open'];
 			if ($url == "") {
-				$this->error = true;
-				$this->errormsg = "No url is given";
-				return !$this->error;
+				$this->error = "No url is given";
+				return false;
 			}
 			if ($this->open) {
 				$this->location = $url . "/qisserver/services2/OpenCourseService";
 			} else {
 				$this->location = $url . "/qisserver/services2/CourseService";
 			}
-			$this->serverID = $serverID;
+			$this->serverId = $serverId;
 		} else {
-			$this->error = true;
-			$this->errormsg = "wrong credentials";
+			$this->error = "wrong credentials";
 			return false;
 		}
 
@@ -36,19 +34,20 @@ class CourseBackend_HisInOne extends CourseBackend
 
 	public function checkConnection()
 	{
-		if ($this->location == "") {
-			$this->error = true;
-			$this->errormsg = "Credentials are not set";
+		if (empty($this->location)) {
+			$this->error = "Credentials are not set";
+		} else {
+			$this->findUnit(123456789, true);
 		}
-		$this->findUnit(190);
-		return !$this->error;
+		return $this->error === false;
 	}
 
 	/**
-	 * @param $roomID int
-	 * @return array|bool if successful an array with the subjectIDs that take place in the room
+	 * @param int $roomId his in one room id to get
+	 * @param bool $connectionCheckOnly true will only check if no soapError is returned, return value will be empty
+	 * @return array|bool if successful an array with the event ids that take place in the room
 	 */
-	public function findUnit($roomID)
+	public function findUnit($roomId, $connectionCheckOnly = false)
 	{
 		$termYear = date('Y');
 		$termType1 = date('n');
@@ -76,40 +75,44 @@ class CourseBackend_HisInOne extends CourseBackend
 		$envelope->appendChild($body);
 		$findUnit = $doc->createElement('ns1:findUnit');
 		$body->appendChild($findUnit);
-		$termYearN = $doc->createElement('termYear', $termYear);
-		$findUnit->appendChild($termYearN);
+		$findUnit->appendChild($doc->createElement('termYear', $termYear));
 		if ($termType1 != 3 && $termType1 != 10) {
-			$termTypeValueId = $doc->createElement('termTypeValueId', $termType);
-			$findUnit->appendChild($termTypeValueId);
+			$findUnit->appendChild($doc->createElement('termTypeValueId', $termType));
 		}
-		$roomIdN = $doc->createElement('ns1:roomId', $roomID);
-		$findUnit->appendChild($roomIdN);
+		$findUnit->appendChild($doc->createElement('ns1:roomId', $roomId));
 
 		$soap_request = $doc->saveXML();
 		$response1 = $this->__doRequest($soap_request, "findUnit");
-		$id = [];
-		if ($this->error == true) {
+		if ($this->error !== false) {
 			return false;
 		}
 		$response2 = $this->toArray($response1);
-		if ($response2 === false) {
+		if (!is_array($response2)) {
+			if ($this->error === false) {
+				$this->error = 'Cannot convert XML response to array';
+			}
+			return false;
+		}
+		if (!isset($response2['soapenvBody'])) {
+			$this->error = 'findUnit(' . $roomId . '): Backend reply is missing element soapenvBody';
 			return false;
 		}
 		if (isset($response2['soapenvBody']['soapenvFault'])) {
-			$this->error = true;
-			$this->errormsg = $response2['soapenvBody']['soapenvFault']['faultcode'] . " " . $response2['soapenvBody']['soapenvFault']['faultstring'];
+			$this->error = $response2['soapenvBody']['soapenvFault']['faultcode'] . " " . $response2['soapenvBody']['soapenvFault']['faultstring'];
 			return false;
-		} elseif ($this->open) {
-			$units = $this->getAttributes($response2, 'soapenvBody/hisfindUnitResponse/hisunits/hisunit');
-			foreach ($units as $unit) {
-				$id[] = $unit['hisid'];
-			}
-		} elseif (!$this->open) {
-			$id = $this->getAttributes($response2, 'soapenvBody/hisfindUnitResponse/hisunitIds/hisid');
+		}
+		// We only need to check if the connection is working (URL ok, credentials ok, ..) so bail out early
+		if ($connectionCheckOnly) {
+			return array();
+		}
+		if ($this->open) {
+			$path = '/soapenvBody/hisfindUnitResponse/hisunits/hisunit/hisid';
 		} else {
-			$this->error = true;
-			$this->errormsg = "url send a xml in a wrong format";
-			$id = false;
+			$path = '/soapenvBody/hisfindUnitResponse/hisunitIds/hisid';
+		}
+		$id = $this->getAttributes($response2, $path);
+		if ($id === false) {
+			$this->error = 'Cannot find ' . $path;
 		}
 		return $id;
 	}
@@ -168,36 +171,15 @@ class CourseBackend_HisInOne extends CourseBackend
 		$output = curl_exec($soap_do);
 
 		if ($output === false) {
-			$this->error = true;
-			$this->errormsg = 'Curl error: ' . curl_error($soap_do);
+			$this->error = 'Curl error: ' . curl_error($soap_do);
 		} else {
 			$this->error = false;
-			$this->errormsg = "";
 			///Operation completed successfully
 		}
 		curl_close($soap_do);
 		return $output;
 	}
 
-	/**
-	 * @param $response xml document
-	 * @return bool|array array representation of the xml if possible
-	 */
-	private function toArray($response)
-	{
-		try {
-			$cleanresponse = preg_replace("/(<\/?)(\w+):([^>]*>)/", "$1$2$3", $response);
-			$xml = new SimpleXMLElement($cleanresponse);
-			$array = json_decode(json_encode((array)$xml), true);
-		} catch (Exception $e) {
-			$this->error = true;
-			$this->errormsg = "url did not send a xml";
-			$array = false;
-		}
-		return $array;
-	}
-
-
 	public function getCacheTime()
 	{
 		return 30 * 60;
@@ -223,72 +205,74 @@ class CourseBackend_HisInOne extends CourseBackend
 	}
 
 
-	public function fetchSchedulesInternal($param)
+	public function fetchSchedulesInternal($requestedRoomIds)
 	{
-		if (empty($param)) {
-			$this->error = true;
-			$this->errormsg = 'Internal Error HisInOne';
-			error_log('No roomId was given in HisInOne fetchShedule');
-			return false;
+		if (empty($requestedRoomIds)) {
+			return array();
 		}
 		$tTables = [];
 		//get all eventIDs in a given room
-		$eventIDs = [];
-		foreach ($param as $ID) {
-			$unitID = $this->findUnit($ID);
-			if ($unitID == false) {
+		$eventIds = [];
+		foreach ($requestedRoomIds as $roomId) {
+			$roomEventIds = $this->findUnit($roomId);
+			if ($roomEventIds === false) {
+				error_log($this->error);
 				$this->error = false;
-				error_log($this->errormsg);
+				// TODO: Error gets swallowed
 				continue;
 			}
-			$eventIDs = array_merge($eventIDs, $unitID);
-			$eventIDs = array_unique($eventIDs);
+			$tTables[$roomId] = [];
+			$eventIds = array_merge($eventIds, $roomEventIds);
 		}
-		if (empty($eventIDs)) {
-			foreach ($param as $room) {
-				$tTables[$room] = [];
-			}
+		$eventIds = array_unique($eventIds);
+		if (empty($eventIds)) {
 			return $tTables;
 		}
-		$events = [];
+		$eventDetails = [];
 		//get all information on each event
-		foreach ($eventIDs as $each_event) {
-			$event = $this->readUnit(intval($each_event));
+		foreach ($eventIds as $eventId) {
+			$event = $this->readUnit(intval($eventId));
 			if ($event === false) {
+				error_log($this->error);
 				$this->error = false;
-				error_log($this->errormsg);
+				// TODO: Error gets swallowed
 				continue;
 			}
-			$events[] = $event;
+			$eventDetails = array_merge($eventDetails, $event);
 		}
 		$currentWeek = $this->getCurrentWeekDates();
-		foreach ($param as $room) {
-			$timetable = array();
-			//Here I go over the soapresponse
-			foreach ($events as $event) {
-				$name = $this->getAttributes($event, '/hisunit/hisdefaulttext');
-				if ($name == false) {
-					//if HisInOne has no default text then there is no name
-					$name = [''];
-				}
-				$dates = $this->getAttributes($event,
-					'/hisunit/hisplanelements/hisplanelement/hisplannedDates/hisplannedDate/hisindividualDates/hisindividualDate');
-				foreach ($dates as $date) {
-					$roomID = $this->getAttributes($date, '/hisroomId')[0];
-					$datum = $this->getAttributes($date, '/hisexecutiondate')[0];
-					if (intval($roomID) == $room && in_array($datum, $currentWeek)) {
-						$startTime = $this->getAttributes($date, 'hisstarttime')[0];
-						$endTime = $this->getAttributes($date, 'hisendtime')[0];
-						$json = array(
-							'title' => $name[0],
-							'start' => $datum . " " . $startTime,
-							'end' => $datum . " " . $endTime
-						);
-						array_push($timetable, $json);
-					}
+		foreach ($eventDetails as $event) {
+			foreach (array('/hisdefaulttext',
+							'/hisshorttext',
+							'/hisshortcomment',
+							'/hisplanelements/hisplanelement/hisdefaulttext') as $path) {
+				$name = $this->getAttributes($event, $path);
+				if (!empty($name) && !empty($name[0]))
+					break;
+				$name = false;
+			}
+			if ($name === false) {
+				$name = ['???'];
+			}
+			$unitPlannedDates = $this->getAttributes($event,
+				'/hisplanelements/hisplanelement/hisplannedDates/hisplannedDate/hisindividualDates/hisindividualDate');
+			if ($unitPlannedDates === false) {
+				$this->error = 'Cannot find ./hisplanelements/hisplanelement/hisplannedDates/hisplannedDate/hisindividualDates/hisindividualDate';
+				return false;
+			}
+			foreach ($unitPlannedDates as $plannedDate) {
+				$eventRoomId = $this->getAttributes($plannedDate, '/hisroomId')[0];
+				$eventDate = $this->getAttributes($plannedDate, '/hisexecutiondate')[0];
+				if (in_array($eventRoomId, $requestedRoomIds) && in_array($eventDate, $currentWeek)) {
+					$startTime = $this->getAttributes($plannedDate, '/hisstarttime')[0];
+					$endTime = $this->getAttributes($plannedDate, '/hisendtime')[0];
+					$tTables[$eventRoomId][] = array(
+						'title' => $name[0],
+						'start' => $eventDate . " " . $startTime,
+						'end' => $eventDate . " " . $endTime
+					);
 				}
 			}
-			$tTables[$room] = $timetable;
 		}
 		return $tTables;
 	}
@@ -316,32 +300,25 @@ class CourseBackend_HisInOne extends CourseBackend
 		$envelope->appendChild($body);
 		$readUnit = $doc->createElement('ns1:readUnit');
 		$body->appendChild($readUnit);
-		$unitId = $doc->createElement('ns1:unitId', $unit);
-		$readUnit->appendChild($unitId);
+		$readUnit->appendChild($doc->createElement('ns1:unitId', $unit));
 
 		$soap_request = $doc->saveXML();
 		$response1 = $this->__doRequest($soap_request, "readUnit");
-		if ($response1 == false) {
+		if ($response1 === false) {
 			return false;
 		}
 		$response2 = $this->toArray($response1);
-		if ($response2 != false) {
-			if (isset($response2['soapenvBody']['soapenvFault'])) {
-				$this->error = true;
-				$this->errormsg = 'SOAP-Fault' . $response2['soapenvBody']['soapenvFault']['faultcode'] . " " . $response2['soapenvBody']['soapenvFault']['faultstring'];
-				return false;
-			} elseif (isset($response2['soapenvBody']['hisreadUnitResponse'])) {
-				$this->error = false;
-				$response3 = $response2['soapenvBody']['hisreadUnitResponse'];
-				$this->errormsg = '';
-				return $response3;
-			} else {
-				$this->error = true;
-				$this->errormsg = "wrong url or the url send a xml in the wrong format";
-				return false;
-			}
+		if ($response2 === false)
+			return false;
+		if (!isset($response2['soapenvBody'])) {
+			$this->error = 'findUnit(' . $unit . '): Backend reply is missing element soapenvBody';
+			return false;
+		}
+		if (isset($response2['soapenvBody']['soapenvFault'])) {
+			$this->error = 'SOAP-Fault' . $response2['soapenvBody']['soapenvFault']['faultcode'] . " " . $response2['soapenvBody']['soapenvFault']['faultstring'];
+			return false;
 		}
-		return false;
+		return $this->getAttributes($response2, '/soapenvBody/hisreadUnitResponse/hisunit');
 	}
 
 	/**
@@ -349,12 +326,12 @@ class CourseBackend_HisInOne extends CourseBackend
 	 */
 	private function getCurrentWeekDates()
 	{
-		$DateArray = array();
-		$startdate = strtotime('Now');
+		$returnValue = array();
+		$startDate = time();
 		for ($i = 0; $i <= 7; $i++) {
-			$DateArray[] = date('Y-m-d', strtotime("+ {$i} day", $startdate));
+			$returnValue[] = date('Y-m-d', strtotime("+{$i} day 12:00", $startDate));
 		}
-		return $DateArray;
+		return $returnValue;
 	}
 
 }