From 15b9f61842e899e4463eb6f058228d9ffd631198 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Wed, 23 Dec 2009 09:26:43 -0800 Subject: [PATCH 1/2] Better error notification for Geonames plugin --- plugins/GeonamesPlugin.php | 247 ++++++++++++++++++++----------------- 1 file changed, 136 insertions(+), 111 deletions(-) diff --git a/plugins/GeonamesPlugin.php b/plugins/GeonamesPlugin.php index a750f12426..0d12c1cf70 100644 --- a/plugins/GeonamesPlugin.php +++ b/plugins/GeonamesPlugin.php @@ -86,30 +86,36 @@ class GeonamesPlugin extends Plugin 'lang' => $language, 'type' => 'json'))); - if ($result->isOk()) { - $rj = json_decode($result->getBody()); - if (count($rj->geonames) > 0) { - $n = $rj->geonames[0]; - - $location = new Location(); - - $location->lat = $n->lat; - $location->lon = $n->lng; - $location->names[$language] = $n->name; - $location->location_id = $n->geonameId; - $location->location_ns = self::LOCATION_NS; - - $this->setCache(array('name' => $name, - 'language' => $language), - $location); - - // handled, don't continue processing! - return false; - } + if (!$result->isOk()) { + $this->log(LOG_WARNING, "Error code " . $result->code . + " from " . $this->host . " for $name"); + return true; } - // Continue processing; we don't have the answer - return true; + $rj = json_decode($result->getBody()); + + if (count($rj->geonames) <= 0) { + $this->log(LOG_WARNING, "No results in response from " . + $this->host . " for $name"); + return true; + } + + $n = $rj->geonames[0]; + + $location = new Location(); + + $location->lat = $n->lat; + $location->lon = $n->lng; + $location->names[$language] = $n->name; + $location->location_id = $n->geonameId; + $location->location_ns = self::LOCATION_NS; + + $this->setCache(array('name' => $name, + 'language' => $language), + $location); + + // handled, don't continue processing! + return false; } /** @@ -143,39 +149,47 @@ class GeonamesPlugin extends Plugin array('geonameId' => $id, 'lang' => $language))); - if ($result->isOk()) { + if (!$result->isOk()) { + $this->log(LOG_WARNING, + "Error code " . $result->code . + " from " . $this->host . " for ID $id"); + return false; + } - $rj = json_decode($result->getBody()); + $rj = json_decode($result->getBody()); - if (count($rj->geonames) > 0) { + if (count($rj->geonames) <= 0) { + $this->log(LOG_WARNING, + "No results in response from " . + $this->host . " for ID $id"); + return false; + } - $parts = array(); + $parts = array(); - foreach ($rj->geonames as $level) { - if (in_array($level->fcode, array('PCLI', 'ADM1', 'PPL'))) { - $parts[] = $level->name; - } - } - - $last = $rj->geonames[count($rj->geonames)-1]; - - if (!in_array($level->fcode, array('PCLI', 'ADM1', 'PPL'))) { - $parts[] = $last->name; - } - - $location = new Location(); - - $location->location_id = $last->geonameId; - $location->location_ns = self::LOCATION_NS; - $location->lat = $last->lat; - $location->lon = $last->lng; - $location->names[$language] = implode(', ', array_reverse($parts)); - - $this->setCache(array('id' => $last->geonameId), - $location); + foreach ($rj->geonames as $level) { + if (in_array($level->fcode, array('PCLI', 'ADM1', 'PPL'))) { + $parts[] = $level->name; } } + $last = $rj->geonames[count($rj->geonames)-1]; + + if (!in_array($level->fcode, array('PCLI', 'ADM1', 'PPL'))) { + $parts[] = $last->name; + } + + $location = new Location(); + + $location->location_id = $last->geonameId; + $location->location_ns = self::LOCATION_NS; + $location->lat = $last->lat; + $location->lon = $last->lng; + $location->names[$language] = implode(', ', array_reverse($parts)); + + $this->setCache(array('id' => $last->geonameId), + $location); + // We're responsible for this NAMESPACE; nobody else // can resolve it @@ -217,48 +231,52 @@ class GeonamesPlugin extends Plugin 'lng' => $lon, 'lang' => $language))); - if ($result->isOk()) { - - $rj = json_decode($result->getBody()); - - if (count($rj->geonames) > 0) { - - $n = $rj->geonames[0]; - - $parts = array(); - - $location = new Location(); - - $parts[] = $n->name; - - if (!empty($n->adminName1)) { - $parts[] = $n->adminName1; - } - - if (!empty($n->countryName)) { - $parts[] = $n->countryName; - } - - $location->location_id = $n->geonameId; - $location->location_ns = self::LOCATION_NS; - $location->lat = $lat; - $location->lon = $lon; - - $location->names[$language] = implode(', ', $parts); - - $this->setCache(array('lat' => $lat, - 'lon' => $lon), - $location); - - // Success! We handled it, so no further processing - - return false; - } + if (!$result->isOk()) { + $this->log(LOG_WARNING, + "Error code " . $result->code . + " from " . $this->host . " for coords $lat, $lon"); + return true; } - // For some reason we don't know, so pass. + $rj = json_decode($result->getBody()); - return true; + if (count($rj->geonames) <= 0) { + $this->log(LOG_WARNING, + "No results in response from " . + $this->host . " for coords $lat, $lon"); + return true; + } + + $n = $rj->geonames[0]; + + $parts = array(); + + $location = new Location(); + + $parts[] = $n->name; + + if (!empty($n->adminName1)) { + $parts[] = $n->adminName1; + } + + if (!empty($n->countryName)) { + $parts[] = $n->countryName; + } + + $location->location_id = $n->geonameId; + $location->location_ns = self::LOCATION_NS; + $location->lat = $lat; + $location->lon = $lon; + + $location->names[$language] = implode(', ', $parts); + + $this->setCache(array('lat' => $lat, + 'lon' => $lon), + $location); + + // Success! We handled it, so no further processing + + return false; } /** @@ -295,37 +313,44 @@ class GeonamesPlugin extends Plugin array('geonameId' => $location->location_id, 'lang' => $language))); - if ($result->isOk()) { + if (!$result->isOk()) { + $this->log(LOG_WARNING, + "Error code " . $result->code . + " from " . $this->host . " for ID " . $location->location_id); + return false; + } - $rj = json_decode($result->getBody()); + $rj = json_decode($result->getBody()); - if (count($rj->geonames) > 0) { + if (count($rj->geonames) <= 0) { + $this->log(LOG_WARNING, + "No results " . + " from " . $this->host . " for ID " . $location->location_id); + return false; + } - $parts = array(); + $parts = array(); - foreach ($rj->geonames as $level) { - if (in_array($level->fcode, array('PCLI', 'ADM1', 'PPL'))) { - $parts[] = $level->name; - } - } - - $last = $rj->geonames[count($rj->geonames)-1]; - - if (!in_array($level->fcode, array('PCLI', 'ADM1', 'PPL'))) { - $parts[] = $last->name; - } - - if (count($parts)) { - $name = implode(', ', array_reverse($parts)); - $this->setCache(array('id' => $location->location_id, - 'language' => $language), - $name); - return false; - } + foreach ($rj->geonames as $level) { + if (in_array($level->fcode, array('PCLI', 'ADM1', 'PPL'))) { + $parts[] = $level->name; } } - return true; + $last = $rj->geonames[count($rj->geonames)-1]; + + if (!in_array($level->fcode, array('PCLI', 'ADM1', 'PPL'))) { + $parts[] = $last->name; + } + + if (count($parts)) { + $name = implode(', ', array_reverse($parts)); + $this->setCache(array('id' => $location->location_id, + 'language' => $language), + $name); + } + + return false; } /** From c0f444f564be0c14a6cd23c4241c6f9cd4331518 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Wed, 23 Dec 2009 12:16:22 -0800 Subject: [PATCH 2/2] make sure Geonames API queries use correct arg separator --- plugins/GeonamesPlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/GeonamesPlugin.php b/plugins/GeonamesPlugin.php index 0d12c1cf70..df99c7849b 100644 --- a/plugins/GeonamesPlugin.php +++ b/plugins/GeonamesPlugin.php @@ -448,7 +448,7 @@ class GeonamesPlugin extends Plugin $params['token'] = $this->token; } - $str = http_build_query($params); + $str = http_build_query($params, null, '&'); return 'http://'.$this->host.'/'.$method.'?'.$str; }