Some minor refactoring.

pear · May 16, 2020 · f4b06c5 · f4b06c5
1 parent a471515
commit f4b06c5
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 32 deletions.
diff --git a/Services/OpenStreetMap/Changeset.php b/Services/OpenStreetMap/Changeset.php
@@ -69,7 +69,6 @@ class Services_OpenStreetMap_Changeset extends Services_OpenStreetMap_Object
      */
     protected $osmChangeXml = null;
 
-
     /**
      * Used to keep track of Id updates.
      *
@@ -84,6 +83,10 @@ class Services_OpenStreetMap_Changeset extends Services_OpenStreetMap_Object
      */
     protected $atomic;
 
+    const ATOMIC = 'atomic';
+    const MESSAGE = 'message';
+    const REVIEW_REQUESTED = 'reviewRequested';
+
     /**
      * Constructor
      *
@@ -97,14 +100,14 @@ class Services_OpenStreetMap_Changeset extends Services_OpenStreetMap_Object
      */
     public function __construct($settings = [])
     {
-        if (array_key_exists('atomic', $settings)) {
-            $this->atomic = $settings['atomic'];
+        if (array_key_exists(self::ATOMIC, $settings)) {
+            $this->atomic = $settings[self::ATOMIC];
         }
-        if (array_key_exists('message', $settings)) {
-            $this->message = $settings['message'];
+        if (array_key_exists(self::MESSAGE, $settings)) {
+            $this->message = $settings[self::MESSAGE];
         }
-        if (array_key_exists('reviewRequested', $settings)) {
-            $this->reviewRequested = $settings['reviewRequested'];
+        if (array_key_exists(self::REVIEW_REQUESTED, $settings)) {
+            $this->reviewRequested = $settings[self::REVIEW_REQUESTED];
         }
     }
 
@@ -133,9 +136,7 @@ public function begin(string $message): void
             . "<changeset id='0' open='false'>"
             . '<tag k="comment" v="' . $message . '"/>'
             . '<tag k="created_by" v="' . $userAgent . '/0.1"/>';
-        if ($this->reviewRequested) {
-            $doc .= '<tag k="review_requested" v="yes"/>';
-        }
+        $doc .= $this->generateReviewRequestedTag();
         $doc .= '</changeset></osm>';
         $url = $config->getValue('server')
             . 'api/'
@@ -210,20 +211,11 @@ public function begin(string $message): void
                 ]
             );
         } else {
-            if ($user !== null && $password === null) {
-                throw new Services_OpenStreetMap_RuntimeException(
-                    "Password must be set"
-                );
-            } elseif ($user === null && $password !== null) {
-                throw new Services_OpenStreetMap_RuntimeException(
-                    "User must be set"
-                );
-            } elseif ($this->getConfig()->getValue('passwordfile') === null) {
-                throw new Services_OpenStreetMap_RuntimeException(
-                    "User & Password for user based auth OR oauth_consumer_key, " .
-                    "oauth_token, consumer_secret, oauth_token_secret have to be " .
-                    "defined to interact with OSM API"
-                );
+            try {
+                $this->_validateCredentials($user, $password);
+            } catch (Exception $ex) {
+                $message = $ex->getMessage();
+                throw new Services_OpenStreetMap_RuntimeException($message);
             }
         }
 
@@ -675,4 +667,47 @@ public function getUpdateMap(): array
     {
         return $this->updateMap;
     }
+
+    /**
+     * Generate the changeset tag to indicate if a review is requested
+     *
+     * @return string
+     */
+    private function generateReviewRequestedTag(): string
+    {
+        $tag = '<tag k="review_requested" v="no"/>';
+        if ($this->reviewRequested) {
+            $tag = '<tag k="review_requested" v="yes"/>';
+        }
+        return $tag;
+    }
+
+    /**
+     * Validate credentials for using the API backend.
+     *
+     * @param string $user     Username
+     * @param string $password Password
+     *
+     * @throws Services_OpenStreetMap_RuntimeException Thrown if user/password or password file not set
+     *
+     * @return void
+     */
+    private function _validateCredentials($user, $password)
+    {
+        if ($user !== null && $password === null) {
+            throw new Services_OpenStreetMap_RuntimeException(
+                "Password must be set"
+            );
+        } elseif ($user === null && $password !== null) {
+            throw new Services_OpenStreetMap_RuntimeException(
+                "User must be set"
+            );
+        } elseif ($this->getConfig()->getValue('passwordfile') === null) {
+            throw new Services_OpenStreetMap_RuntimeException(
+                "User & Password for user based auth OR oauth_consumer_key, " .
+                "oauth_token, consumer_secret, oauth_token_secret have to be " .
+                "defined to interact with OSM API"
+            );
+        }
+    }
 }
diff --git a/Services/OpenStreetMap/OpeningHours.php b/Services/OpenStreetMap/OpeningHours.php
@@ -244,6 +244,7 @@ private function _daySpecToArray(string $day_specification): ?array
     {
         $days = ['mo', 'tu', 'we', 'th', 'fr', 'sa', 'su'];
         $spec = trim(strtolower($day_specification));
+        $retVal = null;
         if ($pos = strpos($spec, '-')) {
             $start_day = substr($spec, 0, $pos);
             $end_day = substr($spec, $pos + 1);
@@ -270,7 +271,7 @@ private function _daySpecToArray(string $day_specification): ?array
             return $days;
         } elseif (strlen($spec) === 2) {
             if (in_array($spec, $days)) {
-                return [$spec];
+                $retVal= [$spec];
             }
         } elseif (strpos($spec, ',')) {
             $delimited = explode(',', $spec);
@@ -280,9 +281,9 @@ private function _daySpecToArray(string $day_specification): ?array
                     $ret[] = $item;
                 }
             }
-            return $ret;
+            $retVal = $ret;
         }
-        return null;
+        return $retVal;
     }
 
     /**
@@ -298,11 +299,7 @@ private function _evaluateOpenEnded(string $time_spec): bool
         $start = $this->_startTime($time_spec);
         $d = getdate($start);
         $ctime = $d['hours'] * 60 + $d['minutes'];
-        if ($ctime < $start) {
-            return false;
-        } else {
-            return true;
-        }
+        return ($ctime >= $start);
     }
 
     /**