From d72498a9c3006210c49cb1690d079d52593db127 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Tue, 14 May 2019 21:20:51 +0200
Subject: [PATCH] [ticket/15987] Go back to previous table definition type

This will still allow using the 'tables' parameter array but will also ensure
full backward compatibility and compatibility with extensions that will add
more tables to the 'tables' array.

PHPBB3-15987
---
 phpBB/config/default/container/tables.yml     | 155 +++++++++---------
 phpBB/phpbb/di/container_builder.php          |   4 +-
 phpBB/phpbb/di/extension/tables.php           |  29 ++--
 tests/di/create_container_test.php            |  12 ++
 .../production/container/environment.yml      |   3 +
 .../config/test/container/environment.yml     |   3 +
 .../ext/vendor/enabled_4/environment.yml      |   2 +
 7 files changed, 115 insertions(+), 93 deletions(-)

diff --git a/phpBB/config/default/container/tables.yml b/phpBB/config/default/container/tables.yml
index eb1e3020a6..4aed35710b 100644
--- a/phpBB/config/default/container/tables.yml
+++ b/phpBB/config/default/container/tables.yml
@@ -1,79 +1,78 @@
 parameters:
-    tables:
-        acl_groups: '%core.table_prefix%acl_groups'
-        acl_options: '%core.table_prefix%acl_options'
-        acl_roles: '%core.table_prefix%acl_roles'
-        acl_roles_data: '%core.table_prefix%acl_roles_data'
-        acl_users: '%core.table_prefix%acl_users'
-        attachments: '%core.table_prefix%attachments'
-        auth_provider_oauth_token_storage: '%core.table_prefix%oauth_tokens'
-        auth_provider_oauth_states: '%core.table_prefix%oauth_states'
-        auth_provider_oauth_account_assoc: '%core.table_prefix%oauth_accounts'
-        banlist: '%core.table_prefix%banlist'
-        bbcodes: '%core.table_prefix%bbcodes'
-        bookmarks: '%core.table_prefix%bookmarks'
-        bots: '%core.table_prefix%bots'
-        captcha_qa_questions: '%core.table_prefix%captcha_questions'
-        captcha_qa_answers: '%core.table_prefix%captcha_answers'
-        captcha_qa_confirm: '%core.table_prefix%qa_confirm'
-        config: '%core.table_prefix%config'
-        config_text: '%core.table_prefix%config_text'
-        confirm: '%core.table_prefix%confirm'
-        disallow: '%core.table_prefix%disallow'
-        drafts: '%core.table_prefix%drafts'
-        ext: '%core.table_prefix%ext'
-        extensions: '%core.table_prefix%extensions'
-        extension_groups: '%core.table_prefix%extension_groups'
-        forums: '%core.table_prefix%forums'
-        forums_access: '%core.table_prefix%forums_access'
-        forums_track: '%core.table_prefix%forums_track'
-        forums_watch: '%core.table_prefix%forums_watch'
-        groups: '%core.table_prefix%groups'
-        icons: '%core.table_prefix%icons'
-        lang: '%core.table_prefix%lang'
-        log: '%core.table_prefix%log'
-        login_attempts: '%core.table_prefix%login_attempts'
-        migrations: '%core.table_prefix%migrations'
-        moderator_cache: '%core.table_prefix%moderator_cache'
-        modules: '%core.table_prefix%modules'
-        notification_types: '%core.table_prefix%notification_types'
-        notifications: '%core.table_prefix%notifications'
-        poll_options: '%core.table_prefix%poll_options'
-        poll_votes: '%core.table_prefix%poll_votes'
-        posts: '%core.table_prefix%posts'
-        privmsgs: '%core.table_prefix%privmsgs'
-        privmsgs_folder: '%core.table_prefix%privmsgs_folder'
-        privmsgs_rules: '%core.table_prefix%privmsgs_rules'
-        privmsgs_to: '%core.table_prefix%privmsgs_to'
-        profile_fields: '%core.table_prefix%profile_fields'
-        profile_fields_data: '%core.table_prefix%profile_fields_data'
-        profile_fields_options_language: '%core.table_prefix%profile_fields_lang'
-        profile_fields_language: '%core.table_prefix%profile_lang'
-        ranks: '%core.table_prefix%ranks'
-        reports: '%core.table_prefix%reports'
-        reports_reasons: '%core.table_prefix%reports_reasons'
-        search_results: '%core.table_prefix%search_results'
-        search_wordlist: '%core.table_prefix%search_wordlist'
-        search_wordmatch: '%core.table_prefix%search_wordmatch'
-        sessions: '%core.table_prefix%sessions'
-        sessions_keys: '%core.table_prefix%sessions_keys'
-        sitelist: '%core.table_prefix%sitelist'
-        smilies: '%core.table_prefix%smilies'
-        sphinx: '%core.table_prefix%sphinx'
-        styles: '%core.table_prefix%styles'
-        styles_template: '%core.table_prefix%styles_template'
-        styles_template_data: '%core.table_prefix%styles_template_data'
-        styles_theme: '%core.table_prefix%styles_theme'
-        styles_imageset: '%core.table_prefix%styles_imageset'
-        styles_imageset_data: '%core.table_prefix%styles_imageset_data'
-        teampage: '%core.table_prefix%teampage'
-        topics: '%core.table_prefix%topics'
-        topics_posted: '%core.table_prefix%topics_posted'
-        topics_track: '%core.table_prefix%topics_track'
-        topics_watch: '%core.table_prefix%topics_watch'
-        user_group: '%core.table_prefix%user_group'
-        user_notifications: '%core.table_prefix%user_notifications'
-        users: '%core.table_prefix%users'
-        warnings: '%core.table_prefix%warnings'
-        words: '%core.table_prefix%words'
-        zebra: '%core.table_prefix%zebra'
+    tables.acl_groups: '%core.table_prefix%acl_groups'
+    tables.acl_options: '%core.table_prefix%acl_options'
+    tables.acl_roles: '%core.table_prefix%acl_roles'
+    tables.acl_roles_data: '%core.table_prefix%acl_roles_data'
+    tables.acl_users: '%core.table_prefix%acl_users'
+    tables.attachments: '%core.table_prefix%attachments'
+    tables.auth_provider_oauth_token_storage: '%core.table_prefix%oauth_tokens'
+    tables.auth_provider_oauth_states: '%core.table_prefix%oauth_states'
+    tables.auth_provider_oauth_account_assoc: '%core.table_prefix%oauth_accounts'
+    tables.banlist: '%core.table_prefix%banlist'
+    tables.bbcodes: '%core.table_prefix%bbcodes'
+    tables.bookmarks: '%core.table_prefix%bookmarks'
+    tables.bots: '%core.table_prefix%bots'
+    tables.captcha_qa_questions: '%core.table_prefix%captcha_questions'
+    tables.captcha_qa_answers: '%core.table_prefix%captcha_answers'
+    tables.captcha_qa_confirm: '%core.table_prefix%qa_confirm'
+    tables.config: '%core.table_prefix%config'
+    tables.config_text: '%core.table_prefix%config_text'
+    tables.confirm: '%core.table_prefix%confirm'
+    tables.disallow: '%core.table_prefix%disallow'
+    tables.drafts: '%core.table_prefix%drafts'
+    tables.ext: '%core.table_prefix%ext'
+    tables.extensions: '%core.table_prefix%extensions'
+    tables.extension_groups: '%core.table_prefix%extension_groups'
+    tables.forums: '%core.table_prefix%forums'
+    tables.forums_access: '%core.table_prefix%forums_access'
+    tables.forums_track: '%core.table_prefix%forums_track'
+    tables.forums_watch: '%core.table_prefix%forums_watch'
+    tables.groups: '%core.table_prefix%groups'
+    tables.icons: '%core.table_prefix%icons'
+    tables.lang: '%core.table_prefix%lang'
+    tables.log: '%core.table_prefix%log'
+    tables.login_attempts: '%core.table_prefix%login_attempts'
+    tables.migrations: '%core.table_prefix%migrations'
+    tables.moderator_cache: '%core.table_prefix%moderator_cache'
+    tables.modules: '%core.table_prefix%modules'
+    tables.notification_types: '%core.table_prefix%notification_types'
+    tables.notifications: '%core.table_prefix%notifications'
+    tables.poll_options: '%core.table_prefix%poll_options'
+    tables.poll_votes: '%core.table_prefix%poll_votes'
+    tables.posts: '%core.table_prefix%posts'
+    tables.privmsgs: '%core.table_prefix%privmsgs'
+    tables.privmsgs_folder: '%core.table_prefix%privmsgs_folder'
+    tables.privmsgs_rules: '%core.table_prefix%privmsgs_rules'
+    tables.privmsgs_to: '%core.table_prefix%privmsgs_to'
+    tables.profile_fields: '%core.table_prefix%profile_fields'
+    tables.profile_fields_data: '%core.table_prefix%profile_fields_data'
+    tables.profile_fields_options_language: '%core.table_prefix%profile_fields_lang'
+    tables.profile_fields_language: '%core.table_prefix%profile_lang'
+    tables.ranks: '%core.table_prefix%ranks'
+    tables.reports: '%core.table_prefix%reports'
+    tables.reports_reasons: '%core.table_prefix%reports_reasons'
+    tables.search_results: '%core.table_prefix%search_results'
+    tables.search_wordlist: '%core.table_prefix%search_wordlist'
+    tables.search_wordmatch: '%core.table_prefix%search_wordmatch'
+    tables.sessions: '%core.table_prefix%sessions'
+    tables.sessions_keys: '%core.table_prefix%sessions_keys'
+    tables.sitelist: '%core.table_prefix%sitelist'
+    tables.smilies: '%core.table_prefix%smilies'
+    tables.sphinx: '%core.table_prefix%sphinx'
+    tables.styles: '%core.table_prefix%styles'
+    tables.styles_template: '%core.table_prefix%styles_template'
+    tables.styles_template_data: '%core.table_prefix%styles_template_data'
+    tables.styles_theme: '%core.table_prefix%styles_theme'
+    tables.styles_imageset: '%core.table_prefix%styles_imageset'
+    tables.styles_imageset_data: '%core.table_prefix%styles_imageset_data'
+    tables.teampage: '%core.table_prefix%teampage'
+    tables.topics: '%core.table_prefix%topics'
+    tables.topics_posted: '%core.table_prefix%topics_posted'
+    tables.topics_track: '%core.table_prefix%topics_track'
+    tables.topics_watch: '%core.table_prefix%topics_watch'
+    tables.user_group: '%core.table_prefix%user_group'
+    tables.user_notifications: '%core.table_prefix%user_notifications'
+    tables.users: '%core.table_prefix%users'
+    tables.warnings: '%core.table_prefix%warnings'
+    tables.words: '%core.table_prefix%words'
+    tables.zebra: '%core.table_prefix%zebra'
diff --git a/phpBB/phpbb/di/container_builder.php b/phpBB/phpbb/di/container_builder.php
index 552b8c7a95..70ceb9b5e3 100644
--- a/phpBB/phpbb/di/container_builder.php
+++ b/phpBB/phpbb/di/container_builder.php
@@ -160,7 +160,6 @@ class container_builder
 			{
 				$this->container_extensions = [
 					new extension\core($this->get_config_path()),
-					new extension\tables(),
 				];
 
 				if ($this->use_extensions)
@@ -168,6 +167,9 @@ class container_builder
 					$this->load_extensions();
 				}
 
+				// Add tables extension after all extensions
+				$this->container_extensions[] = new extension\tables();
+
 				// Inject the config
 				if ($this->config_php_file)
 				{
diff --git a/phpBB/phpbb/di/extension/tables.php b/phpBB/phpbb/di/extension/tables.php
index ff545750b7..40684b6038 100644
--- a/phpBB/phpbb/di/extension/tables.php
+++ b/phpBB/phpbb/di/extension/tables.php
@@ -22,26 +22,27 @@ use Symfony\Component\HttpKernel\DependencyInjection\Extension;
 class tables extends Extension
 {
 	/**
-	 * Loads a specific configuration.
-	 *
-	 * @param array            $configs   An array of configuration values
-	 * @param ContainerBuilder $container A ContainerBuilder instance
-	 *
-	 * @throws \InvalidArgumentException When provided tag is not defined in this extension
+	 * {@inheritDoc}
 	 */
 	public function load(array $configs, ContainerBuilder $container)
 	{
-		if (!$container->hasParameter('tables'))
+		// Tables is a reserved parameter and will be overwritten at all times
+		$tables = [];
+
+		// Add access via 'tables' parameter to acquire array with all tables
+		$parameterBag = $container->getParameterBag();
+		$parameters = $parameterBag->all();
+		foreach ($parameters as $parameter_name => $parameter_value)
 		{
-			return;
+			if (!preg_match('/tables\.(.+)/', $parameter_name, $matches))
+			{
+				continue;
+			}
+
+			$tables[$matches[1]] = $parameter_value;
 		}
 
-		$tables = $container->getParameter('tables');
-
-		foreach ($tables as $table_name => $table_value)
-		{
-			$container->setParameter('tables.' . $table_name, $table_value);
-		}
+		$container->setParameter('tables', $tables);
 	}
 
 	/**
diff --git a/tests/di/create_container_test.php b/tests/di/create_container_test.php
index 8ecad71412..16b49d1f17 100644
--- a/tests/di/create_container_test.php
+++ b/tests/di/create_container_test.php
@@ -77,6 +77,18 @@ namespace
 			$this->assertTrue($container->isFrozen());
 		}
 
+		public function test_tables_mapping()
+		{
+			$this->builder->without_cache();
+			$container = $this->builder->get_container();
+			$this->assertTrue($container->hasParameter('tables'));
+			$tables = $container->getParameter('tables');
+			$this->assertGreaterThan(0, count($tables));
+			$this->assertTrue($container->hasParameter('tables.foo_bar'));
+			$this->assertTrue(isset($tables['foo_bar']));
+			$this->assertEquals($tables['acl_groups'], 'phpbb_some_other');
+		}
+
 		public function test_without_cache()
 		{
 			$this->builder->without_cache();
diff --git a/tests/di/fixtures/config/production/container/environment.yml b/tests/di/fixtures/config/production/container/environment.yml
index 8281d9e941..0af08f0849 100644
--- a/tests/di/fixtures/config/production/container/environment.yml
+++ b/tests/di/fixtures/config/production/container/environment.yml
@@ -1,5 +1,8 @@
 parameters:
     core: true
+    tables.acl_groups: '%core.table_prefix%acl_groups'
+    tables.acl_options: '%core.table_prefix%acl_options'
+    tables.acl_roles: '%core.table_prefix%acl_roles'
 
 services:
     config.php:
diff --git a/tests/di/fixtures/config/test/container/environment.yml b/tests/di/fixtures/config/test/container/environment.yml
index 252117dd32..0a9e4b5e77 100644
--- a/tests/di/fixtures/config/test/container/environment.yml
+++ b/tests/di/fixtures/config/test/container/environment.yml
@@ -1,5 +1,8 @@
 parameters:
     core: true
+    tables.acl_groups: '%core.table_prefix%acl_groups'
+    tables.acl_options: '%core.table_prefix%acl_options'
+    tables.acl_roles: '%core.table_prefix%acl_roles'
 
 services:
     config.php:
diff --git a/tests/di/fixtures/ext/vendor/enabled_4/environment.yml b/tests/di/fixtures/ext/vendor/enabled_4/environment.yml
index d0affe4fd6..d4ed5cbf24 100644
--- a/tests/di/fixtures/ext/vendor/enabled_4/environment.yml
+++ b/tests/di/fixtures/ext/vendor/enabled_4/environment.yml
@@ -1,2 +1,4 @@
 parameters:
     enabled_4: true
+    tables.foo_bar: '%core.table_prefix%foo_bar'
+    tables.acl_groups: '%core.table_prefix%some_other'