EtcdConfig: allow slashes in config key names

Allowing slashes in config key names allows us to trivially support the
proposed hierarchical structure with a single EtcdConfig object, by
fetching values with the relevant prefixes on startup.

Bug: T156924
Change-Id: Ica0914e61baba9c0462481925be15d79b66dc342

diff --git a/autoload.php b/autoload.php
index 3a2ae10..9d205cb 100644 (file)
--- a/autoload.php
+++ b/autoload.php
@@ -437,6 +437,7 @@ $wgAutoloadLocalClasses = [
        'EraseArchivedFile' => __DIR__ . '/maintenance/eraseArchivedFile.php',
        'ErrorPageError' => __DIR__ . '/includes/exception/ErrorPageError.php',
        'EtcdConfig' => __DIR__ . '/includes/config/EtcdConfig.php',
+       'EtcdConfigParseError' => __DIR__ . '/includes/config/EtcdConfigParseError.php',
        'EventRelayer' => __DIR__ . '/includes/libs/eventrelayer/EventRelayer.php',
        'EventRelayerGroup' => __DIR__ . '/includes/EventRelayerGroup.php',
        'EventRelayerKafka' => __DIR__ . '/includes/libs/eventrelayer/EventRelayerKafka.php',

diff --git a/includes/config/EtcdConfig.php b/includes/config/EtcdConfig.php
index d7dc45a..0ec21cb 100644 (file)
--- a/includes/config/EtcdConfig.php
+++ b/includes/config/EtcdConfig.php
@@ -228,7 +228,7 @@ class EtcdConfig implements Config, LoggerAwareInterface {
                // Retrieve all the values under the MediaWiki config directory
                list( $rcode, $rdesc, /* $rhdrs */, $rbody, $rerr ) = $this->http->run( [
                        'method' => 'GET',
-                       'url' => "{$this->protocol}://{$address}/v2/keys/{$this->directory}/",
+                       'url' => "{$this->protocol}://{$address}/v2/keys/{$this->directory}/?recursive=true",
                        'headers' => [ 'content-type' => 'application/json' ]
                ] );
 
@@ -240,28 +240,65 @@ class EtcdConfig implements Config, LoggerAwareInterface {
                                empty( $terminalCodes[$rcode] )
                        ];
                }
+               try {
+                       return [ $this->parseResponse( $rbody ), null, false ];
+               } catch ( EtcdConfigParseError $e ) {
+                       return [ null, $e->getMessage(), false ];
+               }
+       }
 
+       /**
+        * Parse a response body, throwing EtcdConfigParseError if there is a validation error
+        *
+        * @param string $rbody
+        * @return array
+        */
+       protected function parseResponse( $rbody ) {
                $info = json_decode( $rbody, true );
-               if ( $info === null || !isset( $info['node']['nodes'] ) ) {
-                       return [ null, "Unexpected JSON response; missing 'nodes' list.", false ];
+               if ( $info === null ) {
+                       throw new EtcdConfigParseError( "Error unserializing JSON response." );
+               }
+               if ( !isset( $info['node'] ) || !is_array( $info['node'] ) ) {
+                       throw new EtcdConfigParseError(
+                               "Unexpected JSON response: Missing or invalid node at top level." );
                }
-
                $config = [];
-               foreach ( $info['node']['nodes'] as $node ) {
+               $this->parseDirectory( '', $info['node'], $config );
+               return $config;
+       }
+
+       /**
+        * Recursively parse a directory node and populate the array passed by
+        * reference, throwing EtcdConfigParseError if there is a validation error
+        *
+        * @param string $dirName The relative directory name
+        * @param array $dirNode The decoded directory node
+        * @param array &$config The output array
+        */
+       protected function parseDirectory( $dirName, $dirNode, &$config ) {
+               if ( !isset( $dirNode['nodes'] ) ) {
+                       throw new EtcdConfigParseError(
+                               "Unexpected JSON response in dir '$dirName'; missing 'nodes' list." );
+               }
+               if ( !is_array( $dirNode['nodes'] ) ) {
+                       throw new EtcdConfigParseError(
+                               "Unexpected JSON response in dir '$dirName'; 'nodes' is not an array." );
+               }
+
+               foreach ( $dirNode['nodes'] as $node ) {
+                       $baseName = basename( $node['key'] );
+                       $fullName = $dirName === '' ? $baseName : "$dirName/$baseName";
                        if ( !empty( $node['dir'] ) ) {
-                               continue; // skip directories
-                       }
+                               $this->parseDirectory( $fullName, $node, $config );
+                       } else {
+                               $value = $this->unserialize( $node['value'] );
+                               if ( !is_array( $value ) || !array_key_exists( 'val', $value ) ) {
+                                       throw new EtcdConfigParseError( "Failed to parse value for '$fullName'." );
+                               }
 
-                       $name = basename( $node['key'] );
-                       $value = $this->unserialize( $node['value'] );
-                       if ( !is_array( $value ) || !array_key_exists( 'val', $value ) ) {
-                               return [ null, "Failed to parse value for '$name'.", false ];
+                               $config[$fullName] = $value['val'];
                        }
-
-                       $config[$name] = $value['val'];
                }
-
-               return [ $config, null, false ];
        }
 
        /**

diff --git a/includes/config/EtcdConfigParseError.php b/includes/config/EtcdConfigParseError.php
new file mode 100644 (file)
index 0000000..cab90a8
--- /dev/null
+++ b/includes/config/EtcdConfigParseError.php
@@ -0,0 +1,4 @@
+<?php
+
+class EtcdConfigParseError extends Exception {
+}

diff --git a/tests/phpunit/includes/config/EtcdConfigTest.php b/tests/phpunit/includes/config/EtcdConfigTest.php
index c13cf25..ebe1972 100644 (file)
--- a/tests/phpunit/includes/config/EtcdConfigTest.php
+++ b/tests/phpunit/includes/config/EtcdConfigTest.php
@@ -383,7 +383,7 @@ class EtcConfigTest extends PHPUnit_Framework_TestCase {
                                        false // retry
                                ],
                        ],
-                       '200 OK - Skip dir' => [
+                       '200 OK - Empty dir' => [
                                'http' => [
                                        'code' => 200,
                                        'reason' => 'OK',
@@ -395,7 +395,8 @@ class EtcConfigTest extends PHPUnit_Framework_TestCase {
                                                ],
                                                [
                                                        'key' => '/example/sub',
-                                                       'dir' => true
+                                                       'dir' => true,
+                                                       'nodes' => [],
                                                ],
                                                [
                                                        'key' => '/example/bar',
@@ -410,6 +411,68 @@ class EtcConfigTest extends PHPUnit_Framework_TestCase {
                                        false // retry
                                ],
                        ],
+                       '200 OK - Recursive' => [
+                               'http' => [
+                                       'code' => 200,
+                                       'reason' => 'OK',
+                                       'headers' => [],
+                                       'body' => json_encode( [ 'node' => [ 'nodes' => [
+                                               [
+                                                       'key' => '/example/a',
+                                                       'dir' => true,
+                                                       'nodes' => [
+                                                               [
+                                                                       'key' => 'b',
+                                                                       'value' => json_encode( [ 'val' => true ] ),
+                                                               ],
+                                                               [
+                                                                       'key' => 'c',
+                                                                       'value' => json_encode( [ 'val' => false ] ),
+                                                               ],
+                                                       ],
+                                               ],
+                                       ] ] ] ),
+                                       'error' => '',
+                               ],
+                               'expect' => [
+                                       [ 'a/b' => true, 'a/c' => false ], // data
+                                       null,
+                                       false // retry
+                               ],
+                       ],
+                       '200 OK - Missing nodes at second level' => [
+                               'http' => [
+                                       'code' => 200,
+                                       'reason' => 'OK',
+                                       'headers' => [],
+                                       'body' => json_encode( [ 'node' => [ 'nodes' => [
+                                               [
+                                                       'key' => '/example/a',
+                                                       'dir' => true,
+                                               ],
+                                       ] ] ] ),
+                                       'error' => '',
+                               ],
+                               'expect' => [
+                                       null,
+                                       "Unexpected JSON response in dir 'a'; missing 'nodes' list.",
+                                       false // retry
+                               ],
+                       ],
+                       '200 OK - Correctly encoded garbage response' => [
+                               'http' => [
+                                       'code' => 200,
+                                       'reason' => 'OK',
+                                       'headers' => [],
+                                       'body' => json_encode( [ 'foo' => 'bar' ] ),
+                                       'error' => '',
+                               ],
+                               'expect' => [
+                                       null,
+                                       "Unexpected JSON response: Missing or invalid node at top level.",
+                                       false // retry
+                               ],
+                       ],
                        '200 OK - Bad value' => [
                                'http' => [
                                        'code' => 200,
@@ -453,7 +516,7 @@ class EtcConfigTest extends PHPUnit_Framework_TestCase {
                                ],
                                'expect' => [
                                        null, // data
-                                       "Unexpected JSON response; missing 'nodes' list.",
+                                       "Error unserializing JSON response.",
                                        false // retry
                                ],
                        ],
@@ -491,6 +554,9 @@ class EtcConfigTest extends PHPUnit_Framework_TestCase {
        /**
         * @covers EtcdConfig::fetchAllFromEtcdServer
         * @covers EtcdConfig::unserialize
+        * @covers EtcdConfig::parseResponse
+        * @covers EtcdConfig::parseDirectory
+        * @covers EtcdConfigParseError
         * @dataProvider provideFetchFromServer
         */
        public function testFetchFromServer( array $httpResponse, array $expected ) {