GPXIN-32: Fatal error for Route only files when speed etc not suppressed



Issue Information

Issue Type: Bug
 
Priority: Critical
Status: Closed

Reported By:
Ben Tasker
Assigned To:
Ben Tasker
Project: PHP GPXIngest (GPXIN)
Resolution: Fixed (2017-07-04 10:11:03)
Affects Version: 1.02, 1.03,
Target version: 1.03,

Created: 2017-07-02 12:20:50
Time Spent Working


Description
Github #19 reports that in order to successfully import a specific file, the user had to suppress a number of elements

        $gpx->suppress('speed');
        $gpx->suppress('elevation');
        $gpx->suppress('location');
        $gpx->suppress('date');


It's odd that speed (in particular) needed to be suppressed, as GPXIN-16 was supposed to implement auto-detection.

However, the user's GPX file uses a route (support added in GPX-27) so it may be related to that.

Running a quick test, failure to do so doesn't just generate NOTICE level errors, but also results in a fatal:
Fatal error: Cannot access empty property in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 573


Attachments

gpx-ingest-pb.zip

Issue Links

Github #19
Toggle State Changes

Activity


In #19 the user has provided an example of the changes they made to fix the notices:

		if (!$this->suppresselevation){
			if (isset($this->journey->journeys)) {
				$this->journey->journeys->$jkey->segments->$segkey->stats->elevation = new \stdClass();
				$this->journey->journeys->$jkey->segments->$segkey->stats->elevation->max = max($this->jeles);
				$this->journey->journeys->$jkey->segments->$segkey->stats->elevation->min = min($this->jeles);
				$this->journey->journeys->$jkey->segments->$segkey->stats->elevation->avgChange = round(array_sum($this->jeledevs)/count($this->jeledevs),2);
			}
		}

		if (!$this->suppresslocation){
			$this->journey->stats->distanceTravelled = array_sum($this->jdist); // See GPXIN-6

			if (!empty($this->journeylats)) {
				// Update the Lat/Lon bounds. See GPXIN-26
				$this->journey->stats->bounds->Lat->min = min($this->journeylats);
				$this->journey->stats->bounds->Lat->max = max($this->journeylats);
			}
			if (!empty($this->journeylons)) {
				$this->journey->stats->bounds->Lon->min = min($this->journeylons);
				$this->journey->stats->bounds->Lon->max = max($this->journeylons);
			}
		}


(They added the isset and empty checks)

So, looking at that, it's not that speed etc isn't getting suppressed (though elevation and location may not be) but that that section makes the assumption that there'll be a track present. In the given example GPX file, this isn't the case and there's only a route.
btasker added 'gpx-ingest-pb.zip' to Attachments
OK, so the full list of warning's etc without any of that suppressed (and without the user's suggested changes) is
PHP Warning:  min(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 543

Warning: min(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 543
PHP Warning:  max(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 544

Warning: max(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 544
PHP Warning:  max(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 548

Warning: max(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 548
PHP Warning:  min(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 549

Warning: min(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 549
PHP Warning:  max(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 550

Warning: max(): Array must contain at least one element in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 550
PHP Warning:  Division by zero in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 551

Warning: Division by zero in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 551
PHP Notice:  Undefined variable: jkey in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 573

Notice: Undefined variable: jkey in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 573
PHP Fatal error:  Cannot access empty property in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 573

Fatal error: Cannot access empty property in /home/ben/Documents/src.old/PHP-GPX-Ingest/src/GPXIngest/GPXIngest.php on line 573


Looks like there's more than a little bit of tidying to do.
btasker changed priority from 'Major' to 'Critical'
Making bug title more relevant, and bumping priority up to critical as this causes a break in processing.
OK, so the GPXIN-16 auto-suppression isn't firing, because it only fires if a track is present
				foreach ($trkseg->trkpt as $trkpt){

					// Initialise some variables
					$key = "trackpt$x";
					$ptspeed = (int)filter_var($trkpt->desc, FILTER_SANITIZE_NUMBER_INT);
					$speed_string = (string) $trkpt->desc;

					// If speed is not available (and we're not calculating anything which can be used) suppress speed
					// Will need updating in GPXIN-20
					if (!$trkpt->desc && !$this->expisenabled('calcDistance')){
					  $this->suppress('speed'); // Prevent warnings if speed is not available - See GPXIN-16
					}


However, the statements causing the issue, will fire either way. They don't currently process any data from routes though, so the simplest initial fix would be to move them out to a new method and have that check first whether any tracks have been processed


Repo: PHP-GPX-Ingest
Commit: 371916e05b05bffc1bddeb564d183ef622ee5e01
Author: B Tasker <github@<Domain Hidden>>

Date: Mon Jul 03 10:13:15 2017 +0100
Commit Message: Initial fix for GPXIN-32 (Github #19) - Move track stats to new method and only fire if some tracks have been processed

This prevents the fatal error from occurring. There may be other edge cases that can still trigger errors though, so more testing is needed



Modified (-)(+)
-------
src/GPXIngest/GPXIngest.php




Webhook User-Agent

GitHub-Hookshot/18889e1


View Commit

I think it's probably worthy of a separate issue, but might want to consider whether any of the stats are applicable to routes. Most won't be (as we have no spped etc), but the "bounds" may be. (Raised as GPXIN-33)
btasker changed status from 'Open' to 'Resolved'
btasker added 'Fixed' to resolution
btasker changed status from 'Resolved' to 'Closed'