From 5844391fbda38f6c36e40af53a84049e64c66726 Mon Sep 17 00:00:00 2001 From: Sam Reed Date: Wed, 26 Jul 2023 23:00:20 +0100 Subject: [PATCH] import.php: More cleanup * Use default fallback of check_plane() rather than duplicating * Fixup display of IDs in small tags; fixes 7800eb6b38418ac3a1f618066f8e4aace46eaa4d * Rename some variables * Unindent some code * Formatting --- php/import.php | 500 ++++++++++++++++++++++++----------------- test/server/config.php | 2 +- test/server/import.php | 6 +- 3 files changed, 292 insertions(+), 216 deletions(-) diff --git a/php/import.php b/php/import.php index 649d30a0..21256122 100644 --- a/php/import.php +++ b/php/import.php @@ -58,6 +58,12 @@ function text_count($element) { return $xpath->query('.//text()', $element)->length; } +/** + * Trims UTF-8 NBSP. + * + * @param $string string + * @return string + */ function nbsp_trim($string) { return trim($string, "\xC2\xA0"); // UTF-8 NBSP } @@ -71,7 +77,7 @@ function nbsp_trim($string) { * @return array [ Date, color ] */ function check_date($dbh, $type, $date) { - if (strlen($date) == 4) { + if (strlen($date) === 4) { $date = "01.01." . $date; } if (strpos($date, "-") !== false) { @@ -157,109 +163,118 @@ function check_airport($dbh, $code, $name) { * Else match the first word of airline name. * * @param $dbh PDO OpenFlights DB handler - * @param $number string Flight number - * @param $airline string Airline name + * @param $flightNumber string Flight number + * @param $airlineName string Airline name * @param $uid string User ID; unused * @param $history string If "yes", ignore codes and ignore errors * @return array [ Airline ID, airline name, color ] */ -function check_airline($dbh, $number, $airline, $uid, $history) { - $code = substr($number, 0, 2); - $isAlpha = preg_match('/[a-zA-Z0-9]{2}/', $code) && ! preg_match('/\d{2}/', $code); - if ($airline == "" && !$isAlpha) { - $airline = _("Unknown") . "
(" . _("was:") . " " . _("No airline") . ")"; - $color = "#ddf"; - $alid = -1; +function check_airline($dbh, $flightNumber, $airlineName, $uid, $history) { + $code = substr($flightNumber, 0, 2); + $isAlpha = preg_match('/[a-zA-Z0-9]{2}/', $code) && !preg_match('/\d{2}/', $code); + if ($airlineName === "" && !$isAlpha) { + return [ + -1, + _("Unknown") . "
(" . _("was:") . " " . _("No airline") . ")", + "#ddf" + ]; + } + + // Is it alphanumeric characters, but not all numeric characters? Then it's probably an airline code. + if ($isAlpha && $history != "yes") { + $params = [$code]; + $sql = "SELECT name, alias, alid FROM airlines WHERE iata = ? ORDER BY name"; } else { - // Is it alphanumeric characters, but not all numeric characters? Then it's probably an airline code. - if ($isAlpha && $history != "yes") { - $params = [$code]; - $sql = "SELECT name, alias, alid FROM airlines WHERE iata = ? ORDER BY name"; + $airlinepart = explode(' ', $airlineName); + if ($airlinepart[0] == 'Air') { + $part = 'Air ' . $airlinepart[1] . '%'; } else { - $airlinepart = explode(' ', $airline); - if ($airlinepart[0] == 'Air') { - $part = 'Air ' . $airlinepart[1] . '%'; - } else { - $part = $airlinepart[0] . '%'; - } - $params = [$part, $part, $airline]; - $sql = <<prepare($sql); - $sth->execute($params); - - // validate the airline/code against the DB - switch ($sth->rowCount()) { - // No match, add as new if we have a name for it, else return error - case "0": - if ($airline != "") { - $color = "#fdd"; - $alid = -2; - } else { - $color = "#faa"; - $alid = null; - } - break; + } + $sth = $dbh->prepare($sql); + $sth->execute($params); - // Solitary match - case "1": - $dbrow = $sth->fetch(); - if ($airline != "" && (strcasecmp($dbrow['name'], $airline) == 0 || strcasecmp($dbrow['alias'], $airline) == 0)) { - // Exact match - $color = "#fff"; - $airline = $dbrow['name']; - $alid = $dbrow['alid']; - } elseif ($history == "yes") { - // Not an exact match - $color = "#fdd"; - $alid = -2; - } else { - $color = "#ddf"; - $airline = $dbrow['name'] . "
(" . _("was:") . " {$airline})"; - $alid = $dbrow['alid']; - } - break; + // validate the airline/code against the DB + switch ($sth->rowCount()) { + // No match, add as new if we have a name for it, else return error + case 0: + if ($airlineName !== "") { + $color = "#fdd"; + $alid = -2; + } else { + $color = "#faa"; + $alid = null; + } + break; - // Many matches, default to first with a warning if we can't find an exact match - default: + // Solitary match + case 1: + $dbrow = $sth->fetch(); + if ( + $airlineName !== "" && ( + strcasecmp($dbrow['name'], $airlineName) === 0 || + strcasecmp($dbrow['alias'], $airlineName) === 0 + ) + ) { + // Exact match + $color = "#fff"; + $airlineName = $dbrow['name']; + $alid = $dbrow['alid']; + } elseif ($history == "yes") { + // Not an exact match + $color = "#fdd"; + $alid = -2; + } else { $color = "#ddf"; - $first = true; - foreach ($sth as $dbrow) { - $isMatch = $airline != "" && ((strcasecmp($dbrow['name'], $airline) == 0) || - (strcasecmp($dbrow['alias'], $airline) == 0)); - if ($first || $isMatch) { - if ($isMatch) { - $color = "#fff"; - } - if ($first) { - $first = false; - } - $new_airline = $dbrow['name']; - $alid = $dbrow['alid']; + $airlineName = $dbrow['name'] . "
(" . _("was:") . " $airlineName)"; + $alid = $dbrow['alid']; + } + break; + + // Many matches, default to first with a warning if we can't find an exact match + default: + $color = "#ddf"; + $first = true; + foreach ($sth as $dbrow) { + $isMatch = $airlineName !== "" && ( + (strcasecmp($dbrow['name'], $airlineName) === 0) || + (strcasecmp($dbrow['alias'], $airlineName) === 0) + ); + if ($first || $isMatch) { + if ($isMatch) { + $color = "#fff"; } + if ($first) { + $first = false; + } + $newAirline = $dbrow['name']; + $alid = $dbrow['alid']; } - // No match and in historical mode? Add it as new - if ($history == "yes" && $color == "#ddf") { - $color = "#fdd"; - $alid = -2; - } else { - $airline = $new_airline; - } - } + } + // No match and in historical mode? Add it as new + if ($history == "yes" && $color == "#ddf") { + $color = "#fdd"; + $alid = -2; + } else { + $airlineName = $newAirline; + } } - return [$alid, $airline, $color]; + return [$alid, $airlineName, $color]; } /** - * Validate that this plane is in DB + * Validate that this plane is in DB. * * @param $dbh PDO OpenFlights DB handler * @param $plane string Plane ID - * @return array Plane ID, color + * @return array [ Plane ID, color ] */ function check_plane($dbh, $plane) { // If no plane set, return OK @@ -270,7 +285,7 @@ function check_plane($dbh, $plane) { $sql = "SELECT plid FROM planes WHERE name = ?"; $sth = $dbh->prepare($sql); $sth->execute([$plane]); - if ($sth->rowCount() == 1) { + if ($sth->rowCount() === 1) { $plid = $sth->fetchColumn(0); $color = "#fff"; } else { @@ -281,7 +296,7 @@ function check_plane($dbh, $plane) { } /** - * Validate that the importing user owns this trip + * Validate that the importing user owns this trip. * * @param $dbh PDO|null OpenFlights DB handler * @param $uid string User ID @@ -297,7 +312,7 @@ function check_trip($dbh, $uid, $trid = "") { $sql = "SELECT uid FROM trips WHERE trid = ?"; $sth = $dbh->prepare($sql); $sth->execute([$trid]); - if (($sth->rowCount() == 1) && $uid == $sth->fetchColumn(0)) { + if (($sth->rowCount() === 1) && $uid == $sth->fetchColumn(0)) { $color = "#fff"; } else { $color = "#faa"; @@ -306,22 +321,22 @@ function check_trip($dbh, $uid, $trid = "") { } function die_nicely($msg) { - print $msg . "

"; - print ""; + print $msg . "

" + . ""; exit; } -$uploaddir = $_SERVER["DOCUMENT_ROOT"] . '/import/'; +$uploadDir = $_SERVER["DOCUMENT_ROOT"] . '/import/'; $action = $_POST["action"]; switch ($action) { case _("Upload"): - $uploadFile = $uploaddir . basename($_FILES['userfile']['tmp_name']); + $uploadFile = $uploadDir . basename($_FILES['userfile']['tmp_name']); if (move_uploaded_file($_FILES['userfile']['tmp_name'], $uploadFile)) { echo "" . _("Upload successful. Parsing...") . "

" . _("Results") . "

"; flush(); - print "Tmpfile " . basename($_FILES['userfile']['tmp_name']) . "
"; // DEBUG + print "Tmpfile " . basename($_FILES['userfile']['tmp_name']) . "
"; // DEBUG } else { die_nicely("" . _("Upload failed!") . ""); } @@ -330,12 +345,12 @@ function die_nicely($msg) { case _("Import"): $remove_these = [' ','`','"','\'','\\','/']; $filename = $_POST["tmpfile"]; - $uploadFile = $uploaddir . str_replace($remove_these, '', $filename); - if (! file_exists($uploadFile)) { + $uploadFile = $uploadDir . str_replace($remove_these, '', $filename); + if (!file_exists($uploadFile)) { die_nicely(sprintf(_("File %s not found"), $uploadFile)); } print "

" . _("Importing...") . "

"; - print "Tmpfile " . $filename . "
"; // DEBUG + print "Tmpfile " . $filename . "
"; // DEBUG flush(); break; @@ -346,7 +361,7 @@ function die_nicely($msg) { $fileType = $_POST["fileType"]; $history = $_POST["historyMode"] ?? null; $status = ""; -$id_note = false; +$idNote = false; switch ($fileType) { case "FM": @@ -383,12 +398,39 @@ function die_nicely($msg) { if ($action == _("Upload")) { // TODO: probably should be 3px or 3em... - print ""; - print ""; - print ""; + printf( +"
ID" . _("Date") . "" . _("Flight") . "" . _("From") . - "" . _("To") . "" . _("Miles") . "" . _("Time") . "" . - _("Plane") . "" . _("Reg") . "" . _("Seat") . "" . _("Class") . "" . _("Type") . "" . _("Reason") . - "" . _("Trip") . "" . _("Comment") . "
+ + + + + + + + + + + + + + + +", + _("Date"), + _("Flight"), + _("From"), + _("To"), + _("Miles"), + _("Time"), + _("Plane"), + _("Reg"), + _("Seat"), + _("Class"), + _("Type"), + _("Reason"), + _("Trip"), + _("Comment") + ); } $count = 0; @@ -401,13 +443,12 @@ function die_nicely($msg) { // Read and validate date field // - $src_date = nth_text($cols[1], 0); $src_time = nth_text($cols[1], 1); if (strlen($src_time) < 4) { // a stray -1 or +1 is not a time $src_time = null; } - [$src_date, $date_bgcolor] = check_date($dbh, $fileType, $src_date); + [$src_date, $date_bgcolor] = check_date($dbh, $fileType, nth_text($cols[1], 0)); $src_iata = $cols[2]->textContent; $dst_iata = $cols[4]->textContent; @@ -443,8 +484,7 @@ function die_nicely($msg) { // //
%s%s%s%s%s%s%s%s%s%s%s%s%s%s
10-05-2009
06:10
17:35 -1
429 mi
1:27 h
$cells = $row['table td']->elements; - $distance = $cells[0]->textContent; - $distance = str_replace(',', '', nbsp_trim($distance)); + $distance = str_replace(',', '', nbsp_trim($cells[0]->textContent)); $dist_unit = $cells[1]->textContent; if ($dist_unit == "km") { // km to mi @@ -453,129 +493,163 @@ function die_nicely($msg) { $duration = nbsp_trim($cells[2]->textContent); // Airline
number - $airline = nth_text($cols[6], 0); - $number = nth_text($cols[6], 1); - [$alid, $airline, $airline_bgcolor] = check_airline($dbh, $number, $airline, $uid, $history); + $flightNumber = nth_text($cols[6], 1); + [$alid, $airline, $airline_bgcolor] = check_airline( + $dbh, + $flightNumber, + nth_text($cols[6], 0), + $uid, + $history + ); // Load plane model (plid) // Boeing 737-600
LN-RCW
Yngvar Viking // Airbus A319-100 $plane = nth_text($cols[7], 0); - $reg = ''; + $planeRegistration = ''; // See if the text has a registration, but it's optional if (text_count($cols[7]) > 1) { - $reg = nth_text($cols[7], 1); + $planeRegistration = nth_text($cols[7], 1); // We also have a "name" if (text_count($cols[7]) > 2) { - $reg .= " " . nth_text($cols[7], 2); + $planeRegistration .= " " . nth_text($cols[7], 2); } } - if ($plane != "") { - [$plid, $plane_bgcolor] = check_plane($dbh, $plane); - } else { - $plid = null; - $plane_bgcolor = "#fff"; - } + + // If no plane found, it'll return the defaults. + [$plid, $plane_bgcolor] = check_plane($dbh, $plane); // 12A/Window
Economy
Passenger
Business // 2nd field may be blank, so we count fields and offset 1 if it's there $seat = nth_text($cols[8], 0); - [$seatnumber, $seatpos] = explode('/', $seat); - if (text_count($cols[8]) == 4) { - $seatclass = nth_text($cols[8], 1); + [$seatNumber, $seatPos] = explode('/', $seat); + if (text_count($cols[8]) === 4) { + $seatClass = nth_text($cols[8], 1); $offset = 1; } else { - $seatclass = "Economy"; + $seatClass = "Economy"; $offset = 0; } - $seattype = nth_text($cols[8], 1 + $offset); - $seatreason = nth_text($cols[8], 2 + $offset); + + $seatType = nth_text($cols[8], 1 + $offset); + $seatReason = nth_text($cols[8], 2 + $offset); // Com
... $comment = pq($cols[9])->find('span')->attr('title'); - if ($comment && substr($comment, 0, 9) === "Comment: ") { + if ($comment && strpos($comment, "Comment: ") === 0) { $comment = trim(substr($comment, 9)); } - // FM imports don't have a trip, so use fallback values from check_trip() + // FM imports don't have a trip, so this will use fallback values [$trid, $trip_bgcolor] = check_trip(null, ""); break; // case FM case "CSV": $count++; - if ($count == 1) { - // Skip header row - break; + // Skip header row + if ($count === 1) { + continue 2; } + $id = $count - 1; - // 0 Date Time, 1 From, 2 To,3 Flight_Number, 4 Airline_Code, 5 Distance, 6 Duration, - // 7 Seat, 8 Seat_Type, 9 Class, 10 Reason, 11 Plane, 12 Registration, 13 Trip, 14 Note, - // 15 From_Code, 16 To_Code, 17 Airline_Code, 18 Plane_Code - $datetime = explode(' ', $row[0]); + [ + // 0 - Date Time + $datetime, + // 1 - From; Source Airport IATA code + $src_iata, + // 2 - To; Destination Airport IATA code + $dst_iata, + // 3 - Flight_Number + $flightNumber, + // 4 - Airline + $airline, + // 5 - Distance + $distance, + // 6 - Duration + $duration, + // 7 - Seat + $seatNumber, + // 8 - Seat_Type + $seatPos, + // 9 - Class + $seatClass, + // 10 - Reason (for Flight; Work/Leisure/Crew) + $seatReason, + // 11 - Plane + $plane, + // 12 - Registration + $planeRegistration, + // 13 - Trip + $trid, + // 14 - Note + $comment, + // 15 - From_OID - OpenFlights Airport ID for Source Airport + $src_apid, + // 16 - To_OID - OpenFlights Airport ID for Destination Airport + $dst_apid, + // 17 - Airline_OID - OpenFlights Airline ID + $alid, + // 18 - Plane_OID - OpenFlights Plane ID + $plid, + ] = $row; + + $datetime = explode(' ', $datetime); [$src_date, $date_bgcolor] = check_date($dbh, $fileType, $datetime[0]); $src_time = $datetime[1] ?? ""; - $src_iata = $row[1]; - $src_apid = $row[15]; + // Prefer OpenFlight ID if set for relevant rows if ($src_apid) { - $src_iata = "" . sprintf(_('ID'), $src_apid) . ""; + $src_iata = "" . sprintf(_('ID %'), $src_apid) . ""; $src_bgcolor = "#fff"; - $id_note = true; + $idNote = true; } else { [$src_apid, $src_iata, $src_bgcolor] = check_airport($dbh, $src_iata, $src_iata); } - $dst_iata = $row[2]; - $dst_apid = $row[16]; + if ($dst_apid) { - $dst_iata = "" . sprintf(_('ID'), $dst_apid) . ""; + $dst_iata = "" . sprintf(_('ID %'), $dst_apid) . ""; $dst_bgcolor = "#fff"; - $id_note = true; + $idNote = true; } else { [$dst_apid, $dst_iata, $dst_bgcolor] = check_airport($dbh, $dst_iata, $dst_iata); } - $number = $row[3]; - $airline = $row[4]; - $alid = $row[17]; + if ($alid) { - $airline = "" . sprintf(_('ID'), $alid) . ""; + $airline = "" . sprintf(_('ID %'), $alid) . ""; $airline_bgcolor = "#fff"; - $id_note = true; + $idNote = true; } else { - [$alid, $airline, $airline_bgcolor] = check_airline($dbh, $number, $airline, $uid, $history); + [$alid, $airline, $airline_bgcolor] = check_airline($dbh, $flightNumber, $airline, $uid, $history); } - $plane = $row[11]; - $plid = $row[18]; + if ($plid) { - $plane = "" . sprintf(_('ID'), $plid) . ""; + $plane = "" . sprintf(_('ID %'), $plid) . ""; $plane_bgcolor = "#fff"; - $id_note = true; + $idNote = true; } else { [$plid, $plane_bgcolor] = check_plane($dbh, $plane); } - $distance = $row[5]; - $duration = $row[6]; - $seatnumber = $row[7]; - $seatpos = array_search($row[8], POS_MAP); - $seatclass = array_search($row[9], CLASS_MAP); - if ($row[9] == "B") { - $seatclass = "Business"; - } // fix for typo in pre-0.3 versions of spec - $seattype = ""; // This field is not present in CSVs - $seatreason = array_search($row[10], REASON_MAP); - $reg = $row[12]; - [$trid, $trip_bgcolor] = check_trip($dbh, $uid, $row[13]); - $comment = $row[14]; - break; - } + // Get code from mapping + $seatPos = array_search($seatPos, POS_MAP); - // Skip the first row for CSV - if ($fileType == "CSV" && $count == 1) { - continue; + // fix for typo in pre-0.3 versions of spec + if ($seatClass == "B") { + $seatClass = "Business"; + } else { + // Get code from mapping + $seatClass = array_search($seatClass, CLASS_MAP); + } + + $seatType = ""; // This field is not present in CSVs; Passenger + // Get code from mapping + $seatReason = array_search($seatReason, REASON_MAP); + [$trid, $trip_bgcolor] = check_trip($dbh, $uid, $trid); + break; } - //Check if parsing succeeded and tag fatal errors if not + // Check if parsing succeeded and tag fatal errors if not if (!$src_date) { $status = "disabled"; $fatal = "date"; @@ -584,21 +658,21 @@ function die_nicely($msg) { $status = "disabled"; $fatal = "airport"; } else { - if ($duration == "" || $distance == "") { - [$gc_distance, $gc_duration] = gcDistance($dbh, $src_apid, $dst_apid); - } - $duration_bgcolor = "#fff"; $dist_bgcolor = "#fff"; - if ($duration == "") { - $duration = $gc_duration; - $duration_bgcolor = "#ddf"; - } + if ($duration == "" || $distance == "") { + [$gc_distance, $gc_duration] = gcDistance($dbh, $src_apid, $dst_apid); + + if ($duration == "") { + $duration = $gc_duration; + $duration_bgcolor = "#ddf"; + } - if ($distance == "") { - $distance = $gc_distance; - $dist_bgcolor = "#ddf"; + if ($distance == "") { + $distance = $gc_distance; + $dist_bgcolor = "#ddf"; + } } } if (!$alid) { @@ -624,7 +698,8 @@ function die_nicely($msg) { %s %s %s - %s %s + %s + %s %s %s %s @@ -638,7 +713,7 @@ function die_nicely($msg) { $src_time, $airline_bgcolor, $airline, - $number, + $flightNumber, $src_bgcolor, $src_iata, $dst_bgcolor, @@ -649,12 +724,12 @@ function die_nicely($msg) { $duration, $plane_bgcolor, $plane, - $reg, - $seatnumber, - $seatpos, - $seatclass, - $seattype, - $seatreason, + $planeRegistration, + $seatNumber, + $seatPos, + $seatClass, + $seatType, + $seatReason, $trip_bgcolor, $trid, $comment @@ -699,14 +774,9 @@ function die_nicely($msg) { } // Hack to record X-Y and Y-X flights as same in DB - if ($src_apid > $dst_apid) { - $tmp = $src_apid; - $src_apid = $dst_apid; - $dst_apid = $tmp; - $opp = "Y"; - } else { - $opp = "N"; - } + $flip = ($src_apid > $dst_apid); + [$src_apid, $dst_apid] = flip($src_apid, $dst_apid, $flip); + $opp = $flip ? "Y" : "N"; // And now the flight $sql = << - + - + + - - + + + + - + + - +
+

" . - _("Note: This CSV file contains OpenFlights IDs in columns 15-18. These IDs will override the values of any manual changes made to the airport, airline and/or plane columns.") . + _("Note: This CSV file contains OpenFlights IDs in one or more of the columns numbered 15-18 (15 Source Airport ID, 16 Destination Airport ID, 17 Airline ID, 18 Plane ID). These IDs will override the values of any manual changes made to the airport, airline and/or plane columns.") . "
"; } if ($history == "yes") { @@ -777,6 +852,7 @@ function die_nicely($msg) { } if ($status == "disabled") { + // TODO: It's possible to have more than one fatal reason... // TODO: separate : is not i18n friendly print "" . _("Error") . ": "; switch ($fatal) { @@ -821,8 +897,8 @@ function die_nicely($msg) { if ($action == _("Import")) { print "

" . _("Flights successfully imported.") . "


"; print ""; - print ""; + print ""; } diff --git a/test/server/config.php b/test/server/config.php index 984bd1b2..11add5c1 100644 --- a/test/server/config.php +++ b/test/server/config.php @@ -7,7 +7,7 @@ $webroot = 'http://localhost:8080/'; // Path to OpenFlights upload directory -$uploaddir = '../../import/'; +$uploadDir = '../../import/'; // Database configuration $dbhost = "localhost"; diff --git a/test/server/import.php b/test/server/import.php index fcadae02..da675900 100644 --- a/test/server/import.php +++ b/test/server/import.php @@ -133,15 +133,15 @@ public function test() { } function upload_fixture($context, $fixture, $filetype) { - global $webroot, $uploaddir; + global $webroot, $uploadDir; - $context->assertTrue(copy("./fixtures/" . $fixture, $uploaddir . $fixture)); + $context->assertTrue(copy("./fixtures/" . $fixture, $uploadDir . $fixture)); $opts = array('action' => 'Import', 'tmpfile' => $fixture, 'fileType' => $filetype); return $context->post($webroot . "php/import.php", $opts); } function export_to_csv_and_validate($context, $fixture) { - global $webroot, $uploaddir; + global $webroot, $uploadDir; $expected_csv = sort_string(file_get_contents("./fixtures/" . $fixture)); $params = array("export" => "export");