Refactor getArrayizeString/getOptionalArrayizeString

lib/SimpleSAML/Configuration.php

@@ -877,7 +877,7 @@ class Configuration implements Utils\ClearableState
      *
      * @param string $name The name of the option.
      *
-     * @return mixed The option with the given name.
+     * @return array The option with the given name.
      */
     public function getArrayize(string $name): array
     {
@@ -897,10 +897,10 @@ class Configuration implements Utils\ClearableState
      * If the configuration option isn't an array, it will be converted to an array.
      *
      * @param string      $name The name of the option.
-     * @param mixed  $default A default value which will be returned if the option isn't found.
+     * @param array|null  $default A default value which will be returned if the option isn't found.
      *                       The default value can be null or an array.
      *
-     * @return mixed The option with the given name, or $default if the option isn't found and $default is specified.
+     * @return array|null The option with the given name.
      */
     public function getOptionalArrayize(string $name, $default): ?array
     {
@@ -924,33 +924,48 @@ class Configuration implements Utils\ClearableState
      * If the configuration option is a string, it will be converted to an array with a single string
      *
      * @param string $name The name of the option.
-     * @param mixed  $default A default value which will be returned if the option isn't found. The option will be
-     *                       required if this parameter isn't given. The default value can be any value, including
-     *                       null.
+     * @return string[] The option with the given name.
      *
-     * @return mixed The option with the given name, or $default if the option isn't found and $default is specified.
-     *
-     * @throws \Exception If the option is not a string or an array of strings.
+     * @throws \SimpleSAML\Assert\AssertFailedException If the option is not a string or an array of strings.
      */
-    public function getArrayizeString(string $name, $default = self::REQUIRED_OPTION)
+    public function getArrayizeString(string $name): array
     {
-        $ret = $this->getArrayize($name, $default);
+        $ret = $this->getArrayize($name);
+
+        Assert::allString(
+            $ret,
+            sprintf(
+                '%s: The option %s must be a string or an array of strings.',
+                $this->location,
+                var_export($name, true),
+            ),
+        );
 
-        if ($ret === $default) {
-            // the option wasn't found, or it matches the default value. In any case, return this value
         return $ret;
     }
 
-        foreach ($ret as $value) {
-            if (!is_string($value)) {
-                throw new \Exception(
-                    $this->location . ': The option ' . var_export($name, true) .
-                    ' must be a string or an array of strings.'
-                );
-            }
+
+    /**
+     * This function retrieves an optional configuration option with a string or an array of strings.
+     *
+     * If the configuration option is a string, it will be converted to an array with a single string
+     *
+     * @param string         $name The name of the option.
+     * @param string[]|null  $default A default value which will be returned if the option isn't found.
+     *                         The default value can be null or an array of strings.
+     *
+     * @return string[]|null The option with the given name, or $default if the option isn't found and $default is specified.
+     *
+     * @throws \SimpleSAML\Assert\AssertionFailedException If the option is not a string or an array of strings.
+     */
+    public function getOptionalArrayizeString(string $name, ?array $default): ?array
+    {
+        if (!$this->hasValue($name)) {
+            // the option wasn't found, or it matches the default value. In any case, return this value
+            return $default;
         }
 
-        return $ret;
+        return $this->getArrayizeString($name, $allowedValues);
     }
 
 

tests/lib/SimpleSAML/ConfigurationTest.php

@@ -557,10 +557,36 @@ class ConfigurationTest extends ClearStateTestCase
             'opt_int' => 42,
             'opt_str' => 'string',
         ]);
-        $this->assertEquals($c->getArrayize('missing_opt', '--missing--'), '--missing--');
+
+        // Normal use
         $this->assertEquals($c->getArrayize('opt'), ['a', 'b', 'c']);
         $this->assertEquals($c->getArrayize('opt_int'), [42]);
         $this->assertEquals($c->getArrayize('opt_str'), ['string']);
+
+        // Missing option
+        $this->expectException(AssertionFailedException::class);
+        $c->getArrayize('missing_opt');
+    }
+
+
+    /**
+     * Test \SimpleSAML\Configuration::getOptionalArrayize()
+     */
+    public function testGetOptionalArrayize(): void
+    {
+        $c = Configuration::loadFromArray([
+            'opt' => ['a', 'b', 'c'],
+            'opt_int' => 42,
+            'opt_str' => 'string',
+        ]);
+
+        // Normal use
+        $this->assertEquals($c->getOptionalArrayize('opt', ['d']), ['a', 'b', 'c']);
+        $this->assertEquals($c->getOptionalArrayize('opt_int', [1]), [42]);
+        $this->assertEquals($c->getOptionalArrayize('opt_str', ['test']), ['string']);
+
+        // Missing option
+        $this->assertEquals($c->getOptionalArrayize('missing_opt', ['test']), ['test']);
     }
 
 
@@ -572,24 +598,44 @@ class ConfigurationTest extends ClearStateTestCase
         $c = Configuration::loadFromArray([
             'opt' => ['a', 'b', 'c'],
             'opt_str' => 'string',
+            'opt_wrong' => 4,
         ]);
-        $this->assertEquals($c->getArrayizeString('missing_opt', '--missing--'), '--missing--');
+
+        // Normale use
         $this->assertEquals($c->getArrayizeString('opt'), ['a', 'b', 'c']);
         $this->assertEquals($c->getArrayizeString('opt_str'), ['string']);
+
+        // Missing option
+        $this->expectException(AssertionFailedException::class);
+        $c->getArrayizeString('missing_opt');
+
+        // Wrong option
+        $this->expectException(AssertionFailedException::class);
+        $c->getArrayizeString('opt_wrong');
     }
 
 
     /**
-     * Test \SimpleSAML\Configuration::getArrayizeString() option
-     * with an array that contains something that isn't a string.
+     * Test \SimpleSAML\Configuration::getOptionalArrayizeString()
      */
-    public function testGetArrayizeStringWrongValue(): void
+    public function testGetOptionalArrayizeString(): void
     {
-        $this->expectException(Exception::class);
         $c = Configuration::loadFromArray([
-            'opt' => ['a', 'b', 42],
+            'opt' => ['a', 'b', 'c'],
+            'opt_str' => 'string',
+            'opt_wrong' => 4,
         ]);
-        $c->getArrayizeString('opt');
+
+        // Normale use
+        $this->assertEquals($c->getOptionalArrayizeString('opt', ['d']), ['a', 'b', 'c']);
+        $this->assertEquals($c->getOptionalArrayizeString('opt_str', ['test']), ['string']);
+
+        // Missing option
+        $this->assertEquals($c->getOptionalArrayizeString('missing_opt', ['test']), ['test']);
+
+        // Wrong option
+        $this->expectException(AssertionFailedException::class);
+        $c->getOptionalArrayizeString('opt_wrong', ['test']);
     }