From 58ecd3b0483ed0b934964965fb923a3cfc2cb55c Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Fri, 22 Sep 2023 16:52:22 +0200 Subject: [PATCH] [ticket/15699] Rewrite acp_storage PHPBB3-15699 --- .phpunit.result.cache | 1 + phpBB/adm/style/acp_storage.html | 229 +++--- .../style/acp_storage_update_inprogress.html | 32 + .../default/container/services_storage.yml | 17 + phpBB/includes/acp/acp_database.php | 2 +- phpBB/includes/acp/acp_storage.php | 670 +++++++----------- phpBB/includes/functions_user.php | 4 +- phpBB/phpbb/attachment/delete.php | 2 +- phpBB/phpbb/attachment/upload.php | 2 +- phpBB/phpbb/avatar/driver/upload.php | 2 +- .../db/migration/data/v400/storage_track.php | 10 +- phpBB/phpbb/files/filespec_storage.php | 2 +- .../storage/adapter/adapter_interface.php | 14 +- phpBB/phpbb/storage/adapter/local.php | 36 +- phpBB/phpbb/storage/adapter_factory.php | 4 +- phpBB/phpbb/storage/controller/controller.php | 10 +- .../action_in_progress_exception.php | 18 + .../no_action_in_progress_exception.php | 18 + .../{exception.php => storage_exception.php} | 2 +- phpBB/phpbb/storage/file_info.php | 4 +- phpBB/phpbb/storage/helper.php | 192 +++++ phpBB/phpbb/storage/state_helper.php | 215 ++++++ phpBB/phpbb/storage/storage.php | 54 +- phpBB/phpbb/storage/stream_interface.php | 6 +- phpBB/phpbb/storage/update_type.php | 21 + tests/attachment/delete_test.php | 2 +- .../functional/acp_storage_settings_test.php | 6 +- 27 files changed, 954 insertions(+), 621 deletions(-) create mode 100644 .phpunit.result.cache create mode 100644 phpBB/adm/style/acp_storage_update_inprogress.html create mode 100644 phpBB/phpbb/storage/exception/action_in_progress_exception.php create mode 100644 phpBB/phpbb/storage/exception/no_action_in_progress_exception.php rename phpBB/phpbb/storage/exception/{exception.php => storage_exception.php} (96%) create mode 100644 phpBB/phpbb/storage/helper.php create mode 100644 phpBB/phpbb/storage/state_helper.php create mode 100644 phpBB/phpbb/storage/update_type.php diff --git a/.phpunit.result.cache b/.phpunit.result.cache new file mode 100644 index 0000000000..2de2a6daf1 --- /dev/null +++ b/.phpunit.result.cache @@ -0,0 +1 @@ +{"version":1,"defects":[],"times":[]} \ No newline at end of file diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index ca710aa8d5..bb9940554b 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -4,142 +4,117 @@

{{ lang('STORAGE_TITLE') }}

-{% if S_CONTINUE_UPDATING %} - +

{{ lang('STORAGE_TITLE_EXPLAIN') }}

-

{{ lang('CONTINUE_EXPLAIN') }}

+ + + + + + + + + + + {% for storage in STORAGE_STATS %} + + + + + + + {% endfor %} + +
{{ lang('STORAGE_NAME') }}{{ lang('STORAGE_NUM_FILES') }}{{ lang('STORAGE_SIZE') }}{{ lang('STORAGE_FREE') }}
{{ storage.name }}{{ storage.files }}{{ storage.size }}{{ storage.free_space }}
-
-
- {{ lang('SUBMIT') }} -   - - {{ S_FORM_TOKEN }} -
-
-{% else %} -

{{ lang('STORAGE_TITLE_EXPLAIN') }}

- - - - - - - - - - - - {% for storage in STORAGE_STATS %} - - - - - - - {% endfor %} - -
{{ lang('STORAGE_NAME') }}{{ lang('STORAGE_NUM_FILES') }}{{ lang('STORAGE_SIZE') }}{{ lang('STORAGE_FREE') }}
{{ storage.name }}{{ storage.files }}{{ storage.size }}{{ storage.free_space }}
- - {% if S_ERROR %} -
-

{{ lang('WARNING') }}

-

{{ ERROR_MSG }}

-
- {% endif %} - -
- {% for storage in STORAGES %} -
- {{ lang('STORAGE_' ~ storage.get_name | upper ~ '_TITLE') }} -
-

{{ lang('STORAGE_SELECT_DESC') }}
-
- -
-
-
- - {% for provider in PROVIDERS %} - {% if provider.is_available %} -
- {{ lang('STORAGE_' ~ storage.get_name | upper ~ '_TITLE') }} - {{ lang('STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_NAME') }} - {% for name, options in provider.get_options %} - {% set title = 'STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_OPTION_' ~ name | upper %} - {% set description = 'STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_OPTION_' ~ name | upper ~ '_EXPLAIN' %} - {% set input_id = storage.get_name ~ '_' ~ provider.get_name ~ '_' ~ name %} - {% set input_type = options['type'] %} - {% set input_name = storage.get_name ~ '[' ~ name ~ ']' %} - {% set input_value = attribute(config, 'storage\\' ~ storage.get_name ~ '\\config\\' ~ name) %} -
-
- - {% if lang_defined(description) %} -
{{ lang(description) }} - {% endif %} -
-
- {% if input_type in ['text', 'password', 'email'] %} - - {% elseif input_type == 'textarea' %} - - {% elseif input_type == 'radio' %} - {% for option_name, option_value in options['options'] %} - {{ lang(option_name) }} - {% endfor %} - {% elseif input_type == 'select' %} - - {% endif %} -
-
- {% endfor %} -
- {% endif %} - {% endfor %} - {% endfor %} +{% if ERROR_MESSAGES is not empty %} +
+

{{ lang('WARNING') }}

+ {% for ERROR_MESSAGE in ERROR_MESSAGES %} +

{{ ERROR_MESSAGE }}

+ {% endfor %} +
+{% endif %} + + {% for storage in STORAGES %}
+ {{ lang('STORAGE_' ~ storage.get_name | upper ~ '_TITLE') }}
-
+

{{ lang('STORAGE_SELECT_DESC') }}
- - - +
-
- {{ lang('SUBMIT') }} -   - - {{ S_FORM_TOKEN }} -
-
-{% endif %} + {% for provider in PROVIDERS %} + {% if provider.is_available %} +
+ {{ lang('STORAGE_' ~ storage.get_name | upper ~ '_TITLE') }} - {{ lang('STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_NAME') }} + {% for name, options in provider.get_options %} + {% set title = 'STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_OPTION_' ~ name | upper %} + {% set description = 'STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_OPTION_' ~ name | upper ~ '_EXPLAIN' %} + {% set input_id = storage.get_name ~ '_' ~ provider.get_name ~ '_' ~ name %} + {% set input_type = options['type'] %} + {% set input_name = storage.get_name ~ '[' ~ name ~ ']' %} + {% set input_value = attribute(config, 'storage\\' ~ storage.get_name ~ '\\config\\' ~ name) %} +
+
+ + {% if lang_defined(description) %} +
{{ lang(description) }} + {% endif %} +
+
+ {% if input_type in ['text', 'password', 'email'] %} + + {% elseif input_type == 'textarea' %} + + {% elseif input_type == 'radio' %} + {% for option_name, option_value in options['options'] %} + {{ lang(option_name) }} + {% endfor %} + {% elseif input_type == 'select' %} + + {% endif %} +
+
+ {% endfor %} +
+ {% endif %} + {% endfor %} + {% endfor %} + +
+
+
+
+ + + +
+
+
+ +
+ {{ lang('SUBMIT') }} +   + + {{ S_FORM_TOKEN }} +
+ {% include 'overall_footer.html' %} diff --git a/phpBB/adm/style/acp_storage_update_inprogress.html b/phpBB/adm/style/acp_storage_update_inprogress.html new file mode 100644 index 0000000000..9c4b9fd804 --- /dev/null +++ b/phpBB/adm/style/acp_storage_update_inprogress.html @@ -0,0 +1,32 @@ +{% include 'overall_header.html' %} + + + +

{{ lang('STORAGE_TITLE') }}

+ + + +

{{ lang('CONTINUE_EXPLAIN') }}

+ +
+
+ {{ lang('SUBMIT') }} +   + + {{ S_FORM_TOKEN }} +
+
+ +{% include 'overall_footer.html' %} diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index 4ef28ccf3b..401357861d 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -107,3 +107,20 @@ services: - '@storage.attachment' - '@symfony_request' - '@user' + +# Helpers + storage.state_helper: + class: phpbb\storage\state_helper + arguments: + - '@config' + - '@config_text' + - '@storage.provider_collection' + + storage.helper: + class: phpbb\storage\helper + arguments: + - '@config' + - '@storage.provider_collection' + - '@storage.adapter_collection' + - '@storage.adapter.factory' + - '@storage.state_helper' diff --git a/phpBB/includes/acp/acp_database.php b/phpBB/includes/acp/acp_database.php index 7b8dcc3ee5..85943dc5e4 100644 --- a/phpBB/includes/acp/acp_database.php +++ b/phpBB/includes/acp/acp_database.php @@ -287,7 +287,7 @@ class acp_database fclose($fp); fclose($stream); } - catch (\phpbb\storage\exception\exception $e) + catch (\phpbb\storage\exception\storage_exception $e) { trigger_error($user->lang['RESTORE_DOWNLOAD_FAIL'] . adm_back_link($this->u_action)); } diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index d268476b46..996a0f641b 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -11,14 +11,14 @@ * */ -use phpbb\config\db as config; -use phpbb\config\db_text as config_text; use phpbb\db\driver\driver_interface; use phpbb\di\service_collection; use phpbb\language\language; use phpbb\log\log_interface; -use phpbb\path_helper; use phpbb\request\request; +use phpbb\storage\helper; +use phpbb\storage\state_helper; +use phpbb\storage\update_type; use phpbb\template\template; use phpbb\user; @@ -32,12 +32,6 @@ if (!defined('IN_PHPBB')) class acp_storage { - /** @var config $config */ - protected $config; - - /** @var config_text $config_text */ - protected $config_text; - /** @var driver_interface $db */ protected $db; @@ -47,9 +41,6 @@ class acp_storage /** @var log_interface $log */ protected $log; - /** @var path_helper $path_helper */ - protected $path_helper; - /** @var request */ protected $request; @@ -59,9 +50,6 @@ class acp_storage /** @var user */ protected $user; - /** @var service_collection */ - protected $adapter_collection; - /** @var service_collection */ protected $provider_collection; @@ -83,52 +71,44 @@ class acp_storage /** @var string */ public $u_action; - /** @var mixed */ - protected $state; + /** @var state_helper */ + private $state_helper; - /** - * Update type constants - */ - public const STORAGE_UPDATE_TYPE_CONFIG = 0; - public const STORAGE_UPDATE_TYPE_COPY = 1; - public const STORAGE_UPDATE_TYPE_MOVE = 2; + /** @var helper */ + private $storage_helper; /** * @param string $id * @param string $mode */ - public function main(string $id, string $mode) + public function main(string $id, string $mode): void { global $phpbb_container, $phpbb_dispatcher, $phpbb_root_path; - $this->config = $phpbb_container->get('config'); - $this->config_text = $phpbb_container->get('config_text'); $this->db = $phpbb_container->get('dbal.conn'); - $this->filesystem = $phpbb_container->get('filesystem'); $this->lang = $phpbb_container->get('language'); $this->log = $phpbb_container->get('log'); - $this->path_helper = $phpbb_container->get('path_helper'); $this->request = $phpbb_container->get('request'); $this->template = $phpbb_container->get('template'); $this->user = $phpbb_container->get('user'); - $this->adapter_collection = $phpbb_container->get('storage.adapter_collection'); $this->provider_collection = $phpbb_container->get('storage.provider_collection'); $this->storage_collection = $phpbb_container->get('storage.storage_collection'); + $this->filesystem = $phpbb_container->get('filesystem'); $this->phpbb_root_path = $phpbb_root_path; + $this->state_helper = $phpbb_container->get('storage.state_helper'); + $this->storage_helper = $phpbb_container->get('storage.helper'); - // Add necesary language files + // Add necessary language files $this->lang->add_lang(['acp/storage']); /** * Add language strings * * @event core.acp_storage_load - * @since 3.3.0-a1 + * @since 4.0.0-a1 */ $phpbb_dispatcher->trigger_event('core.acp_storage_load'); - @ini_set('memory_limit', '128M'); - switch ($mode) { case 'settings': @@ -141,173 +121,173 @@ class acp_storage * @param string $id * @param string $mode */ - public function settings(string $id, string $mode) + private function settings(string $id, string $mode): void { - $form_key = 'acp_storage'; - add_form_key($form_key); - - // Template from adm/style - $this->tpl_name = 'acp_storage'; - - // Set page title - $this->page_title = 'STORAGE_TITLE'; - $action = $this->request->variable('action', ''); - $this->load_state(); - - // If user cancelled to continue, remove state - if ($this->request->is_set_post('cancel')) + if ($action && !$this->request->is_set_post('cancel')) { - if (!check_form_key($form_key) || !check_link_hash($this->request->variable('hash', ''), 'acp_storage')) + switch ($action) { - trigger_error($this->lang->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); - } + case 'progress_bar': + $this->display_progress_bar(); + break; - if ($this->request->variable('cancel', false)) - { - $action = ''; - $this->state = false; - $this->save_state(); + case 'update': + $this->update_action(); + break; + + default: + trigger_error('NO_ACTION', E_USER_ERROR); } } - - if ($action) + else { - if ($action == 'progress_bar') + // If clicked to cancel (acp_storage_update_progress form) + if ($this->request->is_set_post('cancel')) { - $this->display_progress_bar(); - } - else if ($action != 'update') - { - trigger_error('NO_ACTION', E_USER_ERROR); + $this->state_helper->clear_state(); } - if (!check_link_hash($this->request->variable('hash', ''), 'acp_storage')) + // There is an updating in progress, show the form to continue or cancel + if ($this->state_helper->is_action_in_progress()) { - trigger_error($this->lang->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); + $this->update_inprogress($id, $mode); + } + else + { + $this->settings_form($id, $mode); + } + } + } + + private function update_action(): void + { + // Probably it has sense to disable the forum while this is in progress + + if (!check_link_hash($this->request->variable('hash', ''), 'acp_storage')) + { + trigger_error($this->lang->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); + } + + // If update_type is copy or move, copy files from the old to the new storage + if (in_array($this->state_helper->update_type(), [update_type::COPY, update_type::MOVE], true)) + { + $i = 0; + foreach ($this->state_helper->storages() as $storage_name) + { + // Skip storages that have already copied files + if ($this->state_helper->storage_index() > $i++) + { + continue; + } + + $sql = 'SELECT file_id, file_path + FROM ' . STORAGE_TABLE . " + WHERE storage = '" . $this->db->sql_escape($storage_name) . "' + AND file_id > " . $this->state_helper->file_index(); + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + if (!still_on_time()) + { + $this->db->sql_freeresult($result); + meta_refresh(1, append_sid($this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'))); + // Here could be included the current file compared with the number of total files too + trigger_error($this->lang->lang('STORAGE_UPDATE_REDIRECT', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state_helper->storages()))); + } + + // Copy file from old adapter to the new one + $this->storage_helper->copy_file_to_new_adapter($storage_name, $row['file_path']); + + $this->state_helper->set_file_index($row['file_id']); // update last file index copied + } + + $this->db->sql_freeresult($result); + + // Copied all files of a storage, increase storage index and reset file index + $this->state_helper->set_storage_index($this->state_helper->storage_index()+1); + $this->state_helper->set_file_index(0); } - // If update_type is copy or move, copy files from the old to the new storage - if (in_array($this->state['update_type'], [self::STORAGE_UPDATE_TYPE_COPY, self::STORAGE_UPDATE_TYPE_MOVE], true)) + // If update_type is move files, remove the old files + if ($this->state_helper->update_type() === update_type::MOVE) { $i = 0; - foreach ($this->state['storages'] as $storage_name => $storage_options) + foreach ($this->state_helper->storages() as $storage_name) { // Skip storages that have already moved files - if ($this->state['storage_index'] > $i) + if ($this->state_helper->remove_storage_index() > $i++) { - $i++; continue; } - $current_adapter = $this->get_current_adapter($storage_name); - $new_adapter = $this->get_new_adapter($storage_name); - $sql = 'SELECT file_id, file_path - FROM ' . STORAGE_TABLE . " - WHERE storage = '" . $this->db->sql_escape($storage_name) . "' - AND file_id > " . (int) $this->state['file_index']; + FROM ' . STORAGE_TABLE . " + WHERE storage = '" . $this->db->sql_escape($storage_name) . "' + AND file_id > " . $this->state_helper->file_index(); $result = $this->db->sql_query($sql); while ($row = $this->db->sql_fetchrow($result)) { if (!still_on_time()) { - $this->save_state(); + $this->db->sql_freeresult($result); meta_refresh(1, append_sid($this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'))); - trigger_error($this->lang->lang('self::STORAGE_UPDATE_REDIRECT', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state['storages']))); + trigger_error($this->lang->lang('STORAGE_UPDATE_REMOVE_REDIRECT', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state_helper->storages()))); } - $stream = $current_adapter->read_stream($row['file_path']); - $new_adapter->write_stream($row['file_path'], $stream); + // remove file from old (current) adapter + $current_adapter = $this->storage_helper->get_current_adapter($storage_name); + $current_adapter->delete($row['file_path']); - if (is_resource($stream)) - { - fclose($stream); - } - - $this->state['file_index'] = $row['file_id']; // Set last uploaded file + $this->state_helper->set_file_index($row['file_id']); } - // Copied all files of a storage, increase storage index and reset file index - $this->state['storage_index']++; - $this->state['file_index'] = 0; - } + $this->db->sql_freeresult($result); - // If update_type is move files, remove the old files - if ($this->state['update_type'] === self::STORAGE_UPDATE_TYPE_MOVE) - { - $i = 0; - foreach ($this->state['storages'] as $storage_name => $storage_options) - { - // Skip storages that have already moved files - if ($this->state['remove_storage_index'] > $i) - { - $i++; - continue; - } - - $current_adapter = $this->get_current_adapter($storage_name); - - $sql = 'SELECT file_id, file_path - FROM ' . STORAGE_TABLE . " - WHERE storage = '" . $this->db->sql_escape($storage_name) . "' - AND file_id > " . (int) $this->state['file_index']; - $result = $this->db->sql_query($sql); - - while ($row = $this->db->sql_fetchrow($result)) - { - if (!still_on_time()) - { - $this->save_state(); - meta_refresh(1, append_sid($this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'))); - trigger_error($this->lang->lang('STORAGE_UPDATE_REMOVE_REDIRECT', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state['storages']))); - } - - $current_adapter->delete($row['file_path']); - - $this->state['file_index'] = $row['file_id']; // Set last uploaded file - } - - // Remove all files of a storage, increase storage index and reset file index - $this->state['remove_storage_index']++; - $this->state['file_index'] = 0; - } + // Remove all files of a storage, increase storage index and reset file index + $this->state_helper->set_remove_storage_index($this->state_helper->remove_storage_index()+1); + $this->state_helper->set_file_index(0); } } - - // Here all files have been copied/moved, so save new configuration - foreach (array_keys($this->state['storages']) as $storage_name) - { - $this->update_storage_config($storage_name); - } - - $storages = array_keys($this->state['storages']); - $this->state = false; - $this->save_state(); - - $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_STORAGE_UPDATE', false, $storages); - trigger_error($this->lang->lang('STORAGE_UPDATE_SUCCESSFUL') . adm_back_link($this->u_action) . $this->close_popup_js()); } - // There is an updating in progress, show the form to continue or cancel - if ($this->state != false) + // Here all files have been copied/moved, so save new configuration + foreach ($this->state_helper->storages() as $storage_name) { - $this->template->assign_vars(array( - 'UA_PROGRESS_BAR' => addslashes(append_sid($this->path_helper->get_phpbb_root_path() . $this->path_helper->get_adm_relative_path() . "index." . $this->path_helper->get_php_ext(), "i=$id&mode=$mode&action=progress_bar")), - 'S_CONTINUE_UPDATING' => true, - 'U_CONTINUE_UPDATING' => $this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'), - 'L_CONTINUE' => $this->lang->lang('CONTINUE_UPDATING'), - 'L_CONTINUE_EXPLAIN' => $this->lang->lang('CONTINUE_UPDATING_EXPLAIN'), - )); - - return; + $this->storage_helper->update_storage_config($storage_name); } + $storages = $this->state_helper->storages(); + $this->state_helper->clear_state(); + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_STORAGE_UPDATE', false, [implode(', ', $storages)]); + trigger_error($this->lang->lang('STORAGE_UPDATE_SUCCESSFUL') . adm_back_link($this->u_action) . $this->close_popup_js()); + } + + private function update_inprogress(string $id, string $mode): void + { + // Template from adm/style + $this->tpl_name = 'acp_storage_update_inprogress'; + + // Set page title + $this->page_title = 'STORAGE_TITLE'; + + $this->template->assign_vars(array( + 'UA_PROGRESS_BAR' => addslashes(append_sid($this->u_action, "action=progress_bar")), + 'U_CONTINUE_UPDATING' => $this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'), + 'L_CONTINUE' => $this->lang->lang('CONTINUE_UPDATING'), + 'L_CONTINUE_EXPLAIN' => $this->lang->lang('CONTINUE_UPDATING_EXPLAIN'), + )); + } + + private function settings_form(string $id, string $mode): void + { + $form_key = 'acp_storage'; + add_form_key($form_key); + // Process form and create a "state" for the update, // then show a confirm form - $messages = []; - if ($this->request->is_set_post('submit')) { if (!check_form_key($form_key) || !check_link_hash($this->request->variable('hash', ''), 'acp_storage')) @@ -315,109 +295,125 @@ class acp_storage trigger_error($this->lang->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); } - $modified_storages = []; + $modified_storages = $this->get_modified_storages(); - foreach ($this->storage_collection as $storage) + // validate submited paths if they are local + $messages = []; + foreach ($modified_storages as $storage_name) { - $storage_name = $storage->get_name(); - - $options = $this->get_provider_options($this->get_current_provider($storage_name)); - - $this->validate_path($storage_name, $options, $messages); - - $modified = false; - - // Check if provider have been modified - if ($this->request->variable([$storage_name, 'provider'], '') != $this->get_current_provider($storage_name)) - { - $modified = true; - } - - // Check if options have been modified - if (!$modified) - { - foreach (array_keys($options) as $definition) - { - if ($this->request->variable([$storage_name, $definition], '') != $this->get_current_definition($storage_name, $definition)) - { - $modified = true; - break; - } - } - } - - // If the storage have been modified, validate options - if ($modified) - { - $modified_storages[] = $storage_name; - $this->validate_data($storage_name, $messages); - } + $this->validate_data($storage_name, $messages); + } + if (!empty($messages)) + { + trigger_error(implode('
', $messages) . adm_back_link($this->u_action), E_USER_WARNING); } + // Start process and show form if (!empty($modified_storages)) { - if (empty($messages)) - { - // Create state - $this->state = [ - // Save the value of the checkbox, to remove all files from the - // old storage once they have been successfully moved - 'update_type' => (int) $this->request->variable('update_type', self::STORAGE_UPDATE_TYPE_CONFIG), - 'storage_index' => 0, - 'file_index' => 0, - 'remove_storage_index' => 0, - ]; + // Create state + $this->state_helper->init(update_type::from((int) $this->request->variable('update_type', update_type::CONFIG->value)), $modified_storages, $this->request); - // Save in the state the selected storages and their configuration - foreach ($modified_storages as $storage_name) - { - $this->state['storages'][$storage_name]['provider'] = $this->request->variable([$storage_name, 'provider'], ''); + // Show the confirmation form to start the process + $this->template->assign_vars(array( + 'UA_PROGRESS_BAR' => addslashes(append_sid($this->u_action, "action=progress_bar")), + 'S_CONTINUE_UPDATING' => true, + 'U_CONTINUE_UPDATING' => $this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'), + 'L_CONTINUE' => $this->lang->lang('START_UPDATING'), + 'L_CONTINUE_EXPLAIN' => $this->lang->lang('START_UPDATING_EXPLAIN'), + )); - $options = $this->get_provider_options($this->request->variable([$storage_name, 'provider'], '')); + // Template from adm/style + $this->tpl_name = 'acp_storage_update_inprogress'; - foreach (array_keys($options) as $definition) - { - $this->state['storages'][$storage_name]['config'][$definition] = $this->request->variable([$storage_name, $definition], ''); - } - } + // Set page title + $this->page_title = 'STORAGE_TITLE'; - $this->save_state(); // A storage update is going to be done here - - // Show the confirmation form to start the process - $this->template->assign_vars(array( - 'UA_PROGRESS_BAR' => addslashes(append_sid($this->path_helper->get_phpbb_root_path() . $this->path_helper->get_adm_relative_path() . "index." . $this->path_helper->get_php_ext(), "i=$id&mode=$mode&action=progress_bar")), // same - 'S_CONTINUE_UPDATING' => true, - 'U_CONTINUE_UPDATING' => $this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'), - 'L_CONTINUE' => $this->lang->lang('START_UPDATING'), - 'L_CONTINUE_EXPLAIN' => $this->lang->lang('START_UPDATING_EXPLAIN'), - )); - - return; - } - else - { - trigger_error(implode('
', $messages) . adm_back_link($this->u_action), E_USER_WARNING); - } + return; } // If there is no changes trigger_error($this->lang->lang('STORAGE_NO_CHANGES') . adm_back_link($this->u_action), E_USER_WARNING); } + // Template from adm/style + $this->tpl_name = 'acp_storage'; + + // Set page title + $this->page_title = 'STORAGE_TITLE'; + + $this->storage_stats(); // Show table with storage stats + + // Validate local paths to check if everything is fine + $messages = []; + foreach ($this->storage_collection as $storage) + { + $this->validate_path($storage->get_name(), $messages); + } + + $this->template->assign_vars([ + 'STORAGES' => $this->storage_collection, + 'PROVIDERS' => $this->provider_collection, + + 'ERROR_MESSAGES' => $messages, + + 'U_ACTION' => $this->u_action . '&hash=' . generate_link_hash('acp_storage'), + + 'STORAGE_UPDATE_TYPE_CONFIG' => update_type::CONFIG->value, + 'STORAGE_UPDATE_TYPE_COPY' => update_type::COPY->value, + 'STORAGE_UPDATE_TYPE_MOVE' => update_type::MOVE->value, + ]); + } + + private function get_modified_storages(): array + { + $modified_storages = []; + + foreach ($this->storage_collection as $storage) + { + $storage_name = $storage->get_name(); + $options = $this->storage_helper->get_provider_options($this->storage_helper->get_current_provider($storage_name)); + + $modified = false; + + // Check if provider have been modified + if ($this->request->variable([$storage_name, 'provider'], '') != $this->storage_helper->get_current_provider($storage_name)) + { + $modified = true; + } + else + { + // Check if options have been modified + foreach (array_keys($options) as $definition) + { + if ($this->request->variable([$storage_name, $definition], '') != $this->storage_helper->get_current_definition($storage_name, $definition)) + { + $modified = true; + break; + } + } + } + + if ($modified) + { + $modified_storages[] = $storage_name; + } + } + + return $modified_storages; + } + + protected function storage_stats() + { // Top table with stats of each storage $storage_stats = []; foreach ($this->storage_collection as $storage) { - $storage_name = $storage->get_name(); - $options = $this->get_provider_options($this->get_current_provider($storage_name)); - - $this->validate_path($storage_name, $options, $messages); - try { $free_space = get_formatted_filesize($storage->free_space()); } - catch (\phpbb\storage\exception\exception $e) + catch (\phpbb\storage\exception\storage_exception $e) { $free_space = $this->lang->lang('STORAGE_UNKNOWN'); } @@ -431,18 +427,7 @@ class acp_storage } $this->template->assign_vars([ - 'STORAGES' => $this->storage_collection, - 'STORAGE_STATS' => $storage_stats, - 'PROVIDERS' => $this->provider_collection, - - 'ERROR_MSG' => implode('
', $messages), - 'S_ERROR' => !empty($messages), - - 'U_ACTION' => $this->u_action . '&hash=' . generate_link_hash('acp_storage'), - - 'STORAGE_UPDATE_TYPE_CONFIG' => self::STORAGE_UPDATE_TYPE_CONFIG, - 'STORAGE_UPDATE_TYPE_COPY' => self::STORAGE_UPDATE_TYPE_COPY, - 'STORAGE_UPDATE_TYPE_MOVE' => self::STORAGE_UPDATE_TYPE_MOVE, + 'STORAGE_STATS' => $storage_stats, ]); } @@ -453,11 +438,11 @@ class acp_storage { adm_page_header($this->lang->lang('STORAGE_UPDATE_IN_PROGRESS')); $this->template->set_filenames(array( - 'body' => 'progress_bar.html') + 'body' => 'progress_bar.html') ); $this->template->assign_vars(array( - 'L_PROGRESS' => $this->lang->lang('STORAGE_UPDATE_IN_PROGRESS'), - 'L_PROGRESS_EXPLAIN' => $this->lang->lang('STORAGE_UPDATE_IN_PROGRESS_EXPLAIN')) + 'L_PROGRESS' => $this->lang->lang('STORAGE_UPDATE_IN_PROGRESS'), + 'L_PROGRESS_EXPLAIN' => $this->lang->lang('STORAGE_UPDATE_IN_PROGRESS_EXPLAIN')) ); adm_page_footer(); } @@ -476,70 +461,6 @@ class acp_storage "\n"; } - /** - * Save state of storage update - */ - protected function save_state() : void - { - $state = $this->state; - - if ($state == false) - { - $state = []; - } - - $this->config_text->set('storage_update_state', json_encode($state)); - } - - /** - * Load state of storage update - */ - protected function load_state() : void - { - $state = json_decode($this->config_text->get('storage_update_state'), true); - - if ($state == null || empty($state)) - { - $state = false; - } - - $this->state = $state; - } - - /** - * Get the current provider from config - * - * @param string $storage_name Storage name - * @return string The current provider - */ - protected function get_current_provider(string $storage_name) : string - { - return $this->config['storage\\' . $storage_name . '\\provider']; - } - - /** - * Get adapter definitions from a provider - * - * @param string $provider Provider class - * @return array Adapter definitions - */ - protected function get_provider_options(string $provider) : array - { - return $this->provider_collection->get_by_class($provider)->get_options(); - } - - /** - * Get the current value of the definition of a storage from config - * - * @param string $storage_name Storage name - * @param string $definition Definition - * @return string Definition value - */ - protected function get_current_definition(string $storage_name, string $definition) : string - { - return $this->config['storage\\' . $storage_name . '\\config\\' . $definition]; - } - /** * Validates data * @@ -569,7 +490,7 @@ class acp_storage } // Check options - $new_options = $this->get_provider_options($this->request->variable([$storage_name, 'provider'], '')); + $new_options = $this->storage_helper->get_provider_options($this->request->variable([$storage_name, 'provider'], '')); foreach ($new_options as $definition_key => $definition_value) { @@ -594,6 +515,20 @@ class acp_storage { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG', $definition_title, $storage_title); } + + if ($provider->get_name() == 'local' && $definition_key == 'path') + { + $path = $value; + + if (empty($path)) + { + $messages[] = $this->lang->lang('STORAGE_PATH_NOT_SET', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE')); + } + else if (!$this->filesystem->exists($this->phpbb_root_path . $path) || !$this->filesystem->is_writable($this->phpbb_root_path . $path)) + { + $messages[] = $this->lang->lang('STORAGE_PATH_NOT_EXISTS', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE')); + } + } break; case 'radio': @@ -608,122 +543,31 @@ class acp_storage } /** - * Updates an storage with the info provided in the form + * Validates path when the filesystem is local * * @param string $storage_name Storage name - */ - protected function update_storage_config(string $storage_name) : void - { - $current_options = $this->get_provider_options($this->get_current_provider($storage_name)); - - // Remove old storage config - foreach (array_keys($current_options) as $definition) - { - $this->config->delete('storage\\' . $storage_name . '\\config\\' . $definition); - } - - // Update provider - $this->config->set('storage\\' . $storage_name . '\\provider', $this->state['storages'][$storage_name]['provider']); - - // Set new storage config - $new_options = $this->get_provider_options($this->state['storages'][$storage_name]['provider']); - - foreach (array_keys($new_options) as $definition) - { - $this->config->set('storage\\' . $storage_name . '\\config\\' . $definition, $this->state['storages'][$storage_name]['config'][$definition]); - } - } - - /** - * Validates path - * - * @param string $storage_name Storage name - * @param array $options Storage provider configuration keys * @param array $messages Error messages array * @return void */ - protected function validate_path(string $storage_name, array $options, array &$messages) : void + protected function validate_path(string $storage_name, array &$messages) : void { - if ($this->provider_collection->get_by_class($this->get_current_provider($storage_name))->get_name() == 'local' && isset($options['path'])) + $current_provider = $this->storage_helper->get_current_provider($storage_name); + $options = $this->storage_helper->get_provider_options($current_provider); + + if ($this->provider_collection->get_by_class($current_provider)->get_name() == 'local' && isset($options['path'])) { - $path = $this->request->is_set_post('submit') ? $this->request->variable([$storage_name, 'path'], '') : $this->get_current_definition($storage_name, 'path'); + $path = $this->storage_helper->get_current_definition($storage_name, 'path'); if (empty($path)) { $messages[] = $this->lang->lang('STORAGE_PATH_NOT_SET', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE')); } - else if (!$this->filesystem->is_writable($this->phpbb_root_path . $path) || !$this->filesystem->exists($this->phpbb_root_path . $path)) + else if (!$this->filesystem->exists($this->phpbb_root_path . $path) || !$this->filesystem->is_writable($this->phpbb_root_path . $path)) { $messages[] = $this->lang->lang('STORAGE_PATH_NOT_EXISTS', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE')); } } } - /** - * Get current storage adapter - * - * @param string $storage_name Storage adapter name - * - * @return object Storage adapter instance - */ - protected function get_current_adapter(string $storage_name): object - { - static $adapters = []; - if (!isset($adapters[$storage_name])) - { - $provider = $this->get_current_provider($storage_name); - $provider_class = $this->provider_collection->get_by_class($provider); - - $adapter = $this->adapter_collection->get_by_class($provider_class->get_adapter_class()); - $definitions = $this->get_provider_options($provider); - - $options = []; - foreach (array_keys($definitions) as $definition) - { - $options[$definition] = $this->get_current_definition($storage_name, $definition); - } - - $adapter->configure($options); - //$adapter->set_storage($storage_name); - - $adapters[$storage_name] = $adapter; - } - - return $adapters[$storage_name]; - } - - /** - * Get new storage adapter - * - * @param string $storage_name - * - * @return object Storage adapter instance - */ - protected function get_new_adapter(string $storage_name) : object - { - static $adapters = []; - - if (!isset($adapters[$storage_name])) - { - $provider = $this->state['storages'][$storage_name]['provider']; - $provider_class = $this->provider_collection->get_by_class($provider); - - $adapter = $this->adapter_collection->get_by_class($provider_class->get_adapter_class()); - $definitions = $this->get_provider_options($provider); - - $options = []; - foreach (array_keys($definitions) as $definition) - { - $options[$definition] = $this->state['storages'][$storage_name]['config'][$definition]; - } - - $adapter->configure($options); - //$adapter->set_storage($storage_name); - - $adapters[$storage_name] = $adapter; - } - - return $adapters[$storage_name]; - } } diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index c41e54098e..737332f4fa 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -1789,7 +1789,7 @@ function avatar_delete($mode, $row, $clean_db = false) return true; } - catch (\phpbb\storage\exception\exception $e) + catch (\phpbb\storage\exception\storage_exception $e) { // Fail is covered by return statement below } @@ -2131,7 +2131,7 @@ function group_correct_avatar($group_id, $old_entry) WHERE group_id = $group_id"; $db->sql_query($sql); } - catch (\phpbb\storage\exception\exception $e) + catch (\phpbb\storage\exception\storage_exception $e) { // If rename fail, dont execute the query } diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index 80fd6b62d4..2620a7a94f 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -464,7 +464,7 @@ class delete return true; } } - catch (\phpbb\storage\exception\exception $exception) + catch (\phpbb\storage\exception\storage_exception $exception) { // Fail is covered by return statement below } diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index d5b961de5f..c0b0c490c4 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -351,7 +351,7 @@ class upload return false; } } - catch (\phpbb\storage\exception\exception $e) + catch (\phpbb\storage\exception\storage_exception $e) { // Do nothing } diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index 14b9e4026c..51425b506f 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -19,7 +19,7 @@ use phpbb\event\dispatcher_interface; use phpbb\files\factory; use phpbb\path_helper; use phpbb\routing\helper; -use phpbb\storage\exception\exception as storage_exception; +use phpbb\storage\exception\storage_exception; use phpbb\storage\storage; /** diff --git a/phpBB/phpbb/db/migration/data/v400/storage_track.php b/phpBB/phpbb/db/migration/data/v400/storage_track.php index 767eee5c0d..7cf05503d6 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_track.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_track.php @@ -14,7 +14,7 @@ namespace phpbb\db\migration\data\v400; use phpbb\db\migration\container_aware_migration; -use phpbb\storage\exception\exception; +use phpbb\storage\exception\storage_exception; use phpbb\storage\storage; class storage_track extends container_aware_migration @@ -97,7 +97,7 @@ class storage_track extends container_aware_migration { $storage->track_file($this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $filename . '.' . $ext); } - catch (exception $e) + catch (storage_exception $e) { // If file doesn't exist, don't track it } @@ -121,7 +121,7 @@ class storage_track extends container_aware_migration { $storage->track_file($row['physical_filename']); } - catch (exception $e) + catch (storage_exception $e) { // If file doesn't exist, don't track it } @@ -132,7 +132,7 @@ class storage_track extends container_aware_migration { $storage->track_file('thumb_' . $row['physical_filename']); } - catch (exception $e) + catch (storage_exception $e) { // If file doesn't exist, don't track it } @@ -157,7 +157,7 @@ class storage_track extends container_aware_migration { $storage->track_file($row['filename']); } - catch (exception $e) + catch (storage_exception $e) { // If file doesn't exist, don't track it } diff --git a/phpBB/phpbb/files/filespec_storage.php b/phpBB/phpbb/files/filespec_storage.php index e775935756..83678718b5 100644 --- a/phpBB/phpbb/files/filespec_storage.php +++ b/phpBB/phpbb/files/filespec_storage.php @@ -453,7 +453,7 @@ class filespec_storage fclose($fp); } } - catch (\phpbb\storage\exception\exception $e) + catch (\phpbb\storage\exception\storage_exception $e) { $this->error[] = $this->language->lang($this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR', $this->destination_file); $this->file_moved = false; diff --git a/phpBB/phpbb/storage/adapter/adapter_interface.php b/phpBB/phpbb/storage/adapter/adapter_interface.php index 5bd6525a73..725b954c2d 100644 --- a/phpBB/phpbb/storage/adapter/adapter_interface.php +++ b/phpBB/phpbb/storage/adapter/adapter_interface.php @@ -13,7 +13,7 @@ namespace phpbb\storage\adapter; -use phpbb\storage\exception\exception; +use phpbb\storage\exception\storage_exception; interface adapter_interface { @@ -29,7 +29,7 @@ interface adapter_interface * * @param string $path * @param string $content - * @throws exception When the file cannot be written + * @throws storage_exception When the file cannot be written */ public function put_contents(string $path, string $content): void; @@ -39,7 +39,7 @@ interface adapter_interface * @param string $path The file to read * * @return string Returns file contents - * @throws exception When cannot read file contents + * @throws storage_exception When cannot read file contents */ public function get_contents(string $path): string; @@ -57,7 +57,7 @@ interface adapter_interface * * @param string $path file/directory to remove * - * @throws exception When removal fails. + * @throws storage_exception When removal fails. */ public function delete(string $path): void; @@ -67,7 +67,7 @@ interface adapter_interface * @param string $path_orig The original file/direcotry * @param string $path_dest The target file/directory * - * @throws exception When file/directory cannot be renamed + * @throws storage_exception When file/directory cannot be renamed */ public function rename(string $path_orig, string $path_dest): void; @@ -77,7 +77,7 @@ interface adapter_interface * @param string $path_orig The original filename * @param string $path_dest The target filename * - * @throws exception When the file cannot be copied + * @throws storage_exception When the file cannot be copied */ public function copy(string $path_orig, string $path_dest): void; @@ -94,7 +94,7 @@ interface adapter_interface * Get space available in bytes * * @return float Returns available space - * @throws exception When unable to retrieve available storage space + * @throws storage_exception When unable to retrieve available storage space */ public function free_space(): float; } diff --git a/phpBB/phpbb/storage/adapter/local.php b/phpBB/phpbb/storage/adapter/local.php index be4b3399d6..9923a4fa44 100644 --- a/phpBB/phpbb/storage/adapter/local.php +++ b/phpBB/phpbb/storage/adapter/local.php @@ -14,7 +14,7 @@ namespace phpbb\storage\adapter; use phpbb\storage\stream_interface; -use phpbb\storage\exception\exception; +use phpbb\storage\exception\storage_exception; use phpbb\filesystem\exception\filesystem_exception; use phpbb\filesystem\filesystem; use phpbb\filesystem\helper as filesystem_helper; @@ -117,7 +117,7 @@ class local implements adapter_interface, stream_interface } catch (filesystem_exception $e) { - throw new exception('STORAGE_CANNOT_WRITE_FILE', $path, array(), $e); + throw new storage_exception('STORAGE_CANNOT_WRITE_FILE', $path, array(), $e); } } @@ -130,7 +130,7 @@ class local implements adapter_interface, stream_interface if ($content === false) { - throw new exception('STORAGE_CANNOT_READ_FILE', $path); + throw new storage_exception('STORAGE_CANNOT_READ_FILE', $path); } return $content; @@ -155,7 +155,7 @@ class local implements adapter_interface, stream_interface } catch (filesystem_exception $e) { - throw new exception('STORAGE_CANNOT_DELETE', $path, array(), $e); + throw new storage_exception('STORAGE_CANNOT_DELETE', $path, array(), $e); } } @@ -172,7 +172,7 @@ class local implements adapter_interface, stream_interface } catch (filesystem_exception $e) { - throw new exception('STORAGE_CANNOT_RENAME', $path_orig, array(), $e); + throw new storage_exception('STORAGE_CANNOT_RENAME', $path_orig, array(), $e); } } @@ -189,7 +189,7 @@ class local implements adapter_interface, stream_interface } catch (filesystem_exception $e) { - throw new exception('STORAGE_CANNOT_COPY', $path_orig, array(), $e); + throw new storage_exception('STORAGE_CANNOT_COPY', $path_orig, array(), $e); } } @@ -198,7 +198,7 @@ class local implements adapter_interface, stream_interface * * @param string $path The directory path * - * @throws exception On any directory creation failure + * @throws storage_exception On any directory creation failure */ protected function create_dir(string $path): void { @@ -208,7 +208,7 @@ class local implements adapter_interface, stream_interface } catch (filesystem_exception $e) { - throw new exception('STORAGE_CANNOT_CREATE_DIR', $path, array(), $e); + throw new storage_exception('STORAGE_CANNOT_CREATE_DIR', $path, array(), $e); } } @@ -217,7 +217,7 @@ class local implements adapter_interface, stream_interface * * @param string $path The file path * - * @throws exception On any directory creation failure + * @throws storage_exception On any directory creation failure */ protected function ensure_directory_exists(string $path): void { @@ -264,7 +264,7 @@ class local implements adapter_interface, stream_interface if (!$stream) { - throw new exception('STORAGE_CANNOT_OPEN_FILE', $path); + throw new storage_exception('STORAGE_CANNOT_OPEN_FILE', $path); } return $stream; @@ -281,13 +281,13 @@ class local implements adapter_interface, stream_interface if (!$stream) { - throw new exception('STORAGE_CANNOT_CREATE_FILE', $path); + throw new storage_exception('STORAGE_CANNOT_CREATE_FILE', $path); } if (stream_copy_to_stream($resource, $stream) === false) { fclose($stream); - throw new exception('STORAGE_CANNOT_COPY_RESOURCE'); + throw new storage_exception('STORAGE_CANNOT_COPY_RESOURCE'); } fclose($stream); @@ -298,10 +298,10 @@ class local implements adapter_interface, stream_interface * * @param string $path The file * - * @throws exception When cannot get size - * * @return array Properties - * @throws exception When cannot get size + * @throws storage_exception When cannot get size + * + * @throws storage_exception When cannot get size * */ public function file_size(string $path): array @@ -310,7 +310,7 @@ class local implements adapter_interface, stream_interface if ($size === null) { - throw new exception('STORAGE_CANNOT_GET_FILESIZE'); + throw new storage_exception('STORAGE_CANNOT_GET_FILESIZE'); } return ['size' => $size]; @@ -392,12 +392,12 @@ class local implements adapter_interface, stream_interface if ($free_space === false) { - throw new exception('STORAGE_CANNOT_GET_FREE_SPACE'); + throw new storage_exception('STORAGE_CANNOT_GET_FREE_SPACE'); } } else { - throw new exception('STORAGE_CANNOT_GET_FREE_SPACE'); + throw new storage_exception('STORAGE_CANNOT_GET_FREE_SPACE'); } return $free_space; diff --git a/phpBB/phpbb/storage/adapter_factory.php b/phpBB/phpbb/storage/adapter_factory.php index 5d08bfa7c2..b358328525 100644 --- a/phpBB/phpbb/storage/adapter_factory.php +++ b/phpBB/phpbb/storage/adapter_factory.php @@ -15,7 +15,7 @@ namespace phpbb\storage; use phpbb\config\config; use phpbb\di\service_collection; -use phpbb\storage\exception\exception; +use phpbb\storage\exception\storage_exception; class adapter_factory { @@ -62,7 +62,7 @@ class adapter_factory if (!$provider->is_available()) { - throw new exception('STORAGE_ADAPTER_NOT_AVAILABLE'); + throw new storage_exception('STORAGE_ADAPTER_NOT_AVAILABLE'); } $adapter = $this->adapters->get_by_class($provider->get_adapter_class()); diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 54f95aa224..d1ea9f29f7 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -16,7 +16,7 @@ namespace phpbb\storage\controller; use phpbb\cache\service; use phpbb\db\driver\driver_interface; use phpbb\exception\http_exception; -use phpbb\storage\exception\exception; +use phpbb\storage\exception\storage_exception; use phpbb\storage\storage; use Symfony\Component\HttpFoundation\Request as symfony_request; use Symfony\Component\HttpFoundation\Response; @@ -63,7 +63,7 @@ class controller * @return Response a Symfony response object * * @throws http_exception when can't access $file - * @throws exception when there is an error reading the file + * @throws storage_exception when there is an error reading the file */ public function handle(string $file): Response { @@ -120,7 +120,7 @@ class controller * @param string $file File path * * @return void - * @throws exception when there is an error reading the file + * @throws storage_exception when there is an error reading the file */ protected function prepare(StreamedResponse $response, string $file): void { @@ -133,7 +133,7 @@ class controller { $content_type = $file_info->get('mimetype'); } - catch (exception $e) + catch (storage_exception $e) { $content_type = 'application/octet-stream'; } @@ -148,7 +148,7 @@ class controller { $response->headers->set('Content-Length', $file_info->get('size')); } - catch (exception $e) + catch (storage_exception $e) { // Just don't send this header } diff --git a/phpBB/phpbb/storage/exception/action_in_progress_exception.php b/phpBB/phpbb/storage/exception/action_in_progress_exception.php new file mode 100644 index 0000000000..1e6f01495e --- /dev/null +++ b/phpBB/phpbb/storage/exception/action_in_progress_exception.php @@ -0,0 +1,18 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\storage\exception; + +class action_in_progress_exception extends storage_exception +{ +} diff --git a/phpBB/phpbb/storage/exception/no_action_in_progress_exception.php b/phpBB/phpbb/storage/exception/no_action_in_progress_exception.php new file mode 100644 index 0000000000..5b5e63fc07 --- /dev/null +++ b/phpBB/phpbb/storage/exception/no_action_in_progress_exception.php @@ -0,0 +1,18 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\storage\exception; + +class no_action_in_progress_exception extends storage_exception +{ +} diff --git a/phpBB/phpbb/storage/exception/exception.php b/phpBB/phpbb/storage/exception/storage_exception.php similarity index 96% rename from phpBB/phpbb/storage/exception/exception.php rename to phpBB/phpbb/storage/exception/storage_exception.php index 8268530c16..08c0cfa4ef 100644 --- a/phpBB/phpbb/storage/exception/exception.php +++ b/phpBB/phpbb/storage/exception/storage_exception.php @@ -15,7 +15,7 @@ namespace phpbb\storage\exception; use phpbb\exception\runtime_exception; -class exception extends runtime_exception +class storage_exception extends runtime_exception { /** * Constructor diff --git a/phpBB/phpbb/storage/file_info.php b/phpBB/phpbb/storage/file_info.php index 2e93846838..3bb6b28131 100644 --- a/phpBB/phpbb/storage/file_info.php +++ b/phpBB/phpbb/storage/file_info.php @@ -13,7 +13,7 @@ namespace phpbb\storage; -use phpbb\storage\exception\exception; +use phpbb\storage\exception\storage_exception; use phpbb\storage\adapter\adapter_interface; class file_info @@ -66,7 +66,7 @@ class file_info { if (!method_exists($this->adapter, 'file_' . $name)) { - throw new exception('STORAGE_METHOD_NOT_IMPLEMENTED'); + throw new storage_exception('STORAGE_METHOD_NOT_IMPLEMENTED'); } $this->properties = array_merge($this->properties, call_user_func([$this->adapter, 'file_' . $name], $this->path)); diff --git a/phpBB/phpbb/storage/helper.php b/phpBB/phpbb/storage/helper.php new file mode 100644 index 0000000000..6a5e2cc28c --- /dev/null +++ b/phpBB/phpbb/storage/helper.php @@ -0,0 +1,192 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\storage; + +use phpbb\config\config; +use phpbb\di\service_collection; + +class helper +{ + /** @var config */ + protected $config; + + /** @var service_collection */ + protected $provider_collection; + + /** @var service_collection */ + protected $adapter_collection; + + /** @var adapter_factory */ + protected $adapter_factory; + + /** @var state_helper */ + protected $state_helper; + + public function __construct(config $config, service_collection $provider_collection, service_collection $adapter_collection, adapter_factory $adapter_factory, state_helper $state_helper) + { + $this->config = $config; + $this->provider_collection = $provider_collection; + $this->adapter_collection = $adapter_collection; + $this->adapter_factory = $adapter_factory; + $this->state_helper = $state_helper; + } + + /** + * Get adapter definitions from a provider + * + * @param string $provider Provider class + * @return array Adapter definitions + */ + public function get_provider_options(string $provider) : array + { + return $this->provider_collection->get_by_class($provider)->get_options(); + } + + /** + * Get the current provider from config + * + * @param string $storage_name Storage name + * @return string The current provider + */ + public function get_current_provider(string $storage_name) : string + { + return (string) $this->config['storage\\' . $storage_name . '\\provider']; + } + + /** + * Get the current value of the definition of a storage from config + * + * @param string $storage_name Storage name + * @param string $definition Definition + * @return string Definition value + */ + public function get_current_definition(string $storage_name, string $definition) : string + { + return (string) $this->config['storage\\' . $storage_name . '\\config\\' . $definition]; + } + + /** + * Get current storage adapter + * + * @param string $storage_name Storage adapter name + * + * @return object Storage adapter instance + */ + public function get_current_adapter(string $storage_name): object + { + static $adapters = []; + + if (!isset($adapters[$storage_name])) + { + $adapters[$storage_name] = $this->adapter_factory->get($storage_name); + } + + return $adapters[$storage_name]; + } + + /** + * Get new storage adapter + * + * @param string $storage_name + * + * @return mixed Storage adapter instance + */ + public function get_new_adapter(string $storage_name) + { + static $adapters = []; + + if (!isset($adapters[$storage_name])) + { + $provider = $this->state_helper->new_provider($storage_name); + $provider_class = $this->provider_collection->get_by_class($provider); + + $adapter = $this->adapter_collection->get_by_class($provider_class->get_adapter_class()); + $definitions = $this->get_provider_options($provider); + + $options = []; + foreach (array_keys($definitions) as $definition) + { + $options[$definition] = $this->state_helper->new_definition_value($storage_name, $definition); + } + + $adapter->configure($options); + + $adapters[$storage_name] = $adapter; + } + + return $adapters[$storage_name]; + } + + public function delete_storage_options(string $storage_name): void + { + $provider = $this->get_current_provider($storage_name); + $options = $this->get_provider_options($provider); + + foreach (array_keys($options) as $definition) + { + $this->config->delete('storage\\' . $storage_name . '\\config\\' . $definition); + } + } + + public function set_storage_provider(string $storage_name, string $provider): void + { + $this->config->set('storage\\' . $storage_name . '\\provider', $provider); + } + + public function set_storage_definition(string $storage_name, string $definition, string $value): void + { + $this->config->set('storage\\' . $storage_name . '\\config\\' . $definition, $value); + } + + public function copy_file_to_new_adapter($storage_name, $file): void + { + $current_adapter = $this->get_current_adapter($storage_name); + $new_adapter = $this->get_new_adapter($storage_name); + + $stream = $current_adapter->read_stream($file); + $new_adapter->write_stream($file, $stream); + + if (is_resource($stream)) + { + fclose($stream); + } + } + + + /** + * Updates a storage with the info provided in the form (that is stored in the state at this point) + * + * @param string $storage_name Storage name + */ + public function update_storage_config(string $storage_name) : void + { + + // Remove old storage config + $this->delete_storage_options($storage_name); + + // Update provider + $new_provider = $this->state_helper->new_provider($storage_name); + $this->set_storage_provider($storage_name, $new_provider); + + // Set new storage config + $new_options = $this->get_provider_options($new_provider); + + foreach (array_keys($new_options) as $definition) + { + $new_definition_value = $this->state_helper->new_definition_value($storage_name, $definition); + $this->set_storage_definition($storage_name, $definition, $new_definition_value); + } + } + +} diff --git a/phpBB/phpbb/storage/state_helper.php b/phpBB/phpbb/storage/state_helper.php new file mode 100644 index 0000000000..cc9de6dff3 --- /dev/null +++ b/phpBB/phpbb/storage/state_helper.php @@ -0,0 +1,215 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\storage; + +use phpbb\config\config; +use phpbb\config\db_text; +use phpbb\di\service_collection; +use phpbb\request\request; +use phpbb\storage\exception\action_in_progress_exception; +use phpbb\storage\exception\no_action_in_progress_exception; + +class state_helper +{ + /** @var config */ + protected $config; + + /** @var db_text $config_text */ + protected $config_text; + + /** @var service_collection */ + protected $provider_collection; + + public function __construct(config $config, db_text $config_text, service_collection $provider_collection) + { + $this->config = $config; + $this->config_text = $config_text; + $this->provider_collection = $provider_collection; + } + + /** + * Returns if there is an action in progress + * + * @return bool + */ + public function is_action_in_progress(): bool + { + return !empty(json_decode($this->config_text->get('storage_update_state'), true)); + } + + public function new_provider(string $storage_name): string + { + $state = $this->load_state(); + + return $state['storages'][$storage_name]['provider']; + } + + public function new_definition_value(string $storage_name, string $definition): string + { + $state = $this->load_state(); + + return $state['storages'][$storage_name]['config'][$definition]; + } + + public function update_type(): update_type + { + $state = $this->load_state(); + + return update_type::from($state['update_type']); + } + + public function storage_index(): int + { + $state = $this->load_state(); + + return $state['storage_index']; + } + + public function set_storage_index(int $storage_index): void + { + $state = $this->load_state(); + + $state['storage_index'] = $storage_index; + + $this->save_state($state); + } + + public function remove_storage_index(): int + { + $state = $this->load_state(); + + return $state['remove_storage_index']; + } + + public function set_remove_storage_index(int $storage_index): void + { + $state = $this->load_state(); + + $state['remove_storage_index'] = $storage_index; + + $this->save_state($state); + } + + public function file_index(): int + { + $state = $this->load_state(); + + return $state['file_index']; + } + + public function set_file_index(int $file_index): void + { + $state = $this->load_state(); + + $state['file_index'] = $file_index; + + $this->save_state($state); + } + + public function storages(): array + { + $state = $this->load_state(); + + return array_keys($state['storages']); + } + + /** + * Start a indexing or delete process. + * + * @param update_type $update_type + * @param array $modified_storages + * @param request $request + * + * @throws action_in_progress_exception If there is an action in progress + * @throws \JsonException + */ + public function init(update_type $update_type, array $modified_storages, request $request): void + { + // Is not possible to start a new process when there is one already running + if ($this->is_action_in_progress()) + { + throw new action_in_progress_exception(); + } + + $state = [ + // Save the value of the checkbox, to remove all files from the + // old storage once they have been successfully moved + 'update_type' => $update_type->value, + 'storages' => [], + 'storage_index' => 0, + 'file_index' => 0, + 'remove_storage_index' => 0, + ]; + + // Save in the state the selected storages and their new configuration + foreach ($modified_storages as $storage_name) + { + $state['storages'][$storage_name] = []; + + $state['storages'][$storage_name]['provider'] = $request->variable([$storage_name, 'provider'], ''); + + $options = $this->provider_collection->get_by_class($request->variable([$storage_name, 'provider'], ''))->get_options(); + + foreach (array_keys($options) as $definition) + { + $state['storages'][$storage_name]['config'][$definition] = $request->variable([$storage_name, $definition], ''); + } + } + + $this->save_state($state); + } + + /** + * Clear the state + * + * @throws \JsonException + */ + public function clear_state(): void + { + $this->save_state([]); + } + + /** + * Load the state from the database + * + * @return array + * + * @throws no_action_in_progress_exception If there is no action in progress + */ + private function load_state(): array + { + // Is not possible to execute an action over state if is empty + if (!$this->is_action_in_progress()) + { + throw new no_action_in_progress_exception(); + } + + return json_decode($this->config_text->get('storage_update_state'), true) ?? []; + } + + /** + * Save the specified state in the database + * + * @param array $state + * + * @throws \JsonException + */ + private function save_state(array $state = []): void + { + $this->config_text->set('storage_update_state', json_encode($state, JSON_THROW_ON_ERROR)); + } + + + +} diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 8e4620ab23..af0c9f0c67 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -15,7 +15,7 @@ namespace phpbb\storage; use phpbb\cache\driver\driver_interface as cache; use phpbb\db\driver\driver_interface as db; -use phpbb\storage\exception\exception; +use phpbb\storage\exception\storage_exception; /** * Experimental @@ -102,14 +102,14 @@ class storage * @param string $path The file to be written to. * @param string $content The data to write into the file. * - * @throws exception When the file already exists + * @throws storage_exception When the file already exists * When the file cannot be written */ public function put_contents($path, $content) { if ($this->exists($path)) { - throw new exception('STORAGE_FILE_EXISTS', $path); + throw new storage_exception('STORAGE_FILE_EXISTS', $path); } $this->get_adapter()->put_contents($path, $content); @@ -121,17 +121,17 @@ class storage * * @param string $path The file to read * - * @throws exception When the file doesn't exist - * When cannot read file contents + * @return string Returns file contents * - * @return string Returns file contents + *@throws storage_exception When the file doesn't exist + * When cannot read file contents * */ public function get_contents($path) { if (!$this->exists($path)) { - throw new exception('STORAGE_FILE_NO_EXIST', $path); + throw new storage_exception('STORAGE_FILE_NO_EXIST', $path); } return $this->get_adapter()->get_contents($path); @@ -155,14 +155,14 @@ class storage * * @param string $path file/directory to remove * - * @throws exception When removal fails + * @throws storage_exception When removal fails * When the file doesn't exist */ public function delete($path) { if (!$this->exists($path)) { - throw new exception('STORAGE_FILE_NO_EXIST', $path); + throw new storage_exception('STORAGE_FILE_NO_EXIST', $path); } $this->get_adapter()->delete($path); @@ -175,7 +175,7 @@ class storage * @param string $path_orig The original file/direcotry * @param string $path_dest The target file/directory * - * @throws exception When the file doesn't exist + * @throws storage_exception When the file doesn't exist * When target exists * When file/directory cannot be renamed */ @@ -183,12 +183,12 @@ class storage { if (!$this->exists($path_orig)) { - throw new exception('STORAGE_FILE_NO_EXIST', $path_orig); + throw new storage_exception('STORAGE_FILE_NO_EXIST', $path_orig); } if ($this->exists($path_dest)) { - throw new exception('STORAGE_FILE_EXISTS', $path_dest); + throw new storage_exception('STORAGE_FILE_EXISTS', $path_dest); } $this->get_adapter()->rename($path_orig, $path_dest); @@ -201,7 +201,7 @@ class storage * @param string $path_orig The original filename * @param string $path_dest The target filename * - * @throws exception When the file doesn't exist + * @throws storage_exception When the file doesn't exist * When target exists * When the file cannot be copied */ @@ -209,12 +209,12 @@ class storage { if (!$this->exists($path_orig)) { - throw new exception('STORAGE_FILE_NO_EXIST', $path_orig); + throw new storage_exception('STORAGE_FILE_NO_EXIST', $path_orig); } if ($this->exists($path_dest)) { - throw new exception('STORAGE_FILE_EXISTS', $path_dest); + throw new storage_exception('STORAGE_FILE_EXISTS', $path_dest); } $this->get_adapter()->copy($path_orig, $path_dest); @@ -226,16 +226,16 @@ class storage * * @param string $path File to read * - * @throws exception When the file doesn't exist + * @return resource Returns a file pointer + *@throws storage_exception When the file doesn't exist * When unable to open file * - * @return resource Returns a file pointer */ public function read_stream($path) { if (!$this->exists($path)) { - throw new exception('STORAGE_FILE_NO_EXIST', $path); + throw new storage_exception('STORAGE_FILE_NO_EXIST', $path); } $stream = null; @@ -262,19 +262,19 @@ class storage * @param string $path The target file * @param resource $resource The resource * - * @throws exception When the file exist + * @throws storage_exception When the file exist * When target file cannot be created */ public function write_stream($path, $resource) { if ($this->exists($path)) { - throw new exception('STORAGE_FILE_EXISTS', $path); + throw new storage_exception('STORAGE_FILE_EXISTS', $path); } if (!is_resource($resource)) { - throw new exception('STORAGE_INVALID_RESOURCE'); + throw new storage_exception('STORAGE_INVALID_RESOURCE'); } $adapter = $this->get_adapter(); @@ -301,7 +301,7 @@ class storage { if (!$this->get_adapter()->exists($path)) { - throw new exception('STORAGE_FILE_NO_EXIST', $path); + throw new storage_exception('STORAGE_FILE_NO_EXIST', $path); } $sql_ary = array( @@ -403,16 +403,16 @@ class storage * * @param string $path The file * - * @throws exception When the adapter doesn't implement the method + * @return \phpbb\storage\file_info Returns file_info object + *@throws storage_exception When the adapter doesn't implement the method * When the file doesn't exist * - * @return \phpbb\storage\file_info Returns file_info object */ public function file_info($path) { if (!$this->exists($path)) { - throw new exception('STORAGE_FILE_NO_EXIST', $path); + throw new storage_exception('STORAGE_FILE_NO_EXIST', $path); } return new file_info($this->get_adapter(), $path); @@ -484,9 +484,9 @@ class storage /** * Get space available in bytes * - * @throws exception When unable to retrieve available storage space + * @return float Returns available space + *@throws storage_exception When unable to retrieve available storage space * - * @return float Returns available space */ public function free_space() { diff --git a/phpBB/phpbb/storage/stream_interface.php b/phpBB/phpbb/storage/stream_interface.php index 9687a2d910..424ffcb95c 100644 --- a/phpBB/phpbb/storage/stream_interface.php +++ b/phpBB/phpbb/storage/stream_interface.php @@ -13,7 +13,7 @@ namespace phpbb\storage; -use phpbb\storage\exception\exception; +use phpbb\storage\exception\storage_exception; interface stream_interface { @@ -23,7 +23,7 @@ interface stream_interface * @param string $path File to read * * @return resource Returns a file pointer - * @throws exception When unable to open file + * @throws storage_exception When unable to open file */ public function read_stream(string $path); @@ -34,7 +34,7 @@ interface stream_interface * @param resource $resource The resource * * @return void - * @throws exception When target file exists + * @throws storage_exception When target file exists * When target file cannot be created */ public function write_stream(string $path, $resource): void; diff --git a/phpBB/phpbb/storage/update_type.php b/phpBB/phpbb/storage/update_type.php new file mode 100644 index 0000000000..32f25ad13d --- /dev/null +++ b/phpBB/phpbb/storage/update_type.php @@ -0,0 +1,21 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\storage; + +enum update_type: int +{ + case CONFIG = 0; + case COPY = 1; + case MOVE = 2; +} diff --git a/tests/attachment/delete_test.php b/tests/attachment/delete_test.php index faf2f960e6..7acac28f33 100644 --- a/tests/attachment/delete_test.php +++ b/tests/attachment/delete_test.php @@ -102,7 +102,7 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case { $this->storage->expects($this->any()) ->method('delete') - ->willThrowException(new \phpbb\storage\exception\exception); + ->willThrowException(new \phpbb\storage\exception\storage_exception); } else { diff --git a/tests/functional/acp_storage_settings_test.php b/tests/functional/acp_storage_settings_test.php index b99e3e8fa9..f8c1728dcb 100644 --- a/tests/functional/acp_storage_settings_test.php +++ b/tests/functional/acp_storage_settings_test.php @@ -68,9 +68,9 @@ class phpbb_functional_acp_storage_settings_test extends phpbb_functional_test_c // Visit ACP Storage settings again - warning should be displayed $crawler = self::request('GET', 'adm/index.php?i=acp_storage&mode=settings&sid=' . $this->sid); $this->assertContainsLang('WARNING', $crawler->filter('div[class="errorbox"] > h3')->text()); - $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_ATTACHMENT_TITLE')), $crawler->filter('div[class="errorbox"] > p')->text()); - $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_AVATAR_TITLE')), $crawler->filter('div[class="errorbox"] > p')->text()); - $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_BACKUP_TITLE')), $crawler->filter('div[class="errorbox"] > p')->text()); + $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_ATTACHMENT_TITLE')), $crawler->filter('div[class="errorbox"]')->text()); + $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_AVATAR_TITLE')), $crawler->filter('div[class="errorbox"]')->text()); + $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_BACKUP_TITLE')), $crawler->filter('div[class="errorbox"]')->text()); // Restore default state $filesystem->chmod($phpbb_root_path . $attachments_storage_path, 777);