From fd2e4592c3fc01c969c752d6da00f4ed7792c03b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Mon, 25 Jun 2018 20:04:13 +0200 Subject: [PATCH 01/34] [ticket/15699] Move files between filesystems PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 9 ++++ phpBB/includes/acp/acp_storage.php | 72 ++++++++++++++++++++++++++++++ phpBB/language/en/acp/storage.php | 20 +++++---- 3 files changed, 92 insertions(+), 9 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 3886f006d1..49535c1a74 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -97,6 +97,15 @@ {% endfor %} {% endfor %} +
+
+

{{ lang('STORAGE_REMOVE_OLD_FILES_EXPLAIN') }}
+
+ +
+
+
+
{{ lang('SUBMIT') }}   diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 3ea1036167..f9e0e75ae0 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -24,6 +24,9 @@ class acp_storage /** @var \phpbb\config\config $config */ protected $config; + /** @var \db\driver\driver_interface $db */ + protected $db; + /** @var \phpbb\language\language $lang */ protected $lang; @@ -33,6 +36,9 @@ class acp_storage /** @var \phpbb\template\template */ protected $template; + /** @var \phpbb\di\service_collection */ + protected $adapter_collection; + /** @var \phpbb\di\service_collection */ protected $provider_collection; @@ -63,10 +69,13 @@ class acp_storage global $phpbb_container, $phpbb_dispatcher, $phpbb_root_path; $this->config = $phpbb_container->get('config'); + $this->db = $phpbb_container->get('dbal.conn'); $this->filesystem = $phpbb_container->get('filesystem'); $this->lang = $phpbb_container->get('language'); $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->phpbb_root_path = $phpbb_root_path; @@ -153,6 +162,31 @@ class acp_storage { foreach ($modified_storages as $storage_name) { + $current_adapter = $this->get_current_adapter($storage_name); + $new_adapter = $this->get_new_adapter($storage_name); + + $sql = 'SELECT file_path + FROM ' . STORAGE_TABLE . " + WHERE storage = '" . $storage_name . "'"; + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $stream = $current_adapter->read_stream($row['file_path']); + $new_adapter->write_stream($row['file_path'], $stream); + fclose($stream); + } + + $this->db->sql_rowseek(0, $result); + + if ($this->request->variable('remove_old', false)) + { + while ($row = $this->db->sql_fetchrow($result)) + { + $current_adapter->delete($row['file_path']); + } + } + $this->update_storage_config($storage_name); } @@ -375,4 +409,42 @@ class acp_storage } } } + + protected function get_current_adapter($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); + + return $adapter; + } + + protected function get_new_adapter($storage_name) + { + $provider = $this->get_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->get_new_definition($storage_name, $definition); + } + + $adapter->configure($options); + + return $adapter; + } } diff --git a/phpBB/language/en/acp/storage.php b/phpBB/language/en/acp/storage.php index 98ad84df53..ae3c007c4a 100644 --- a/phpBB/language/en/acp/storage.php +++ b/phpBB/language/en/acp/storage.php @@ -39,15 +39,17 @@ if (empty($lang) || !is_array($lang)) $lang = array_merge($lang, array( // Template - 'STORAGE_TITLE' => 'Storage Settings', - 'STORAGE_TITLE_EXPLAIN' => 'Change storage providers for the file storage types of phpBB. Choose local or remote providers to store files added to or created by phpBB.', - 'STORAGE_SELECT' => 'Select storage', - 'STORAGE_SELECT_DESC' => 'Select a storage from the list.', - 'STORAGE_NAME' => 'Storage name', - 'STORAGE_NUM_FILES' => 'Number of files', - 'STORAGE_SIZE' => 'Size', - 'STORAGE_FREE' => 'Available space', - 'STORAGE_UNKNOWN' => 'Unknown', + 'STORAGE_TITLE' => 'Storage Settings', + 'STORAGE_TITLE_EXPLAIN' => 'Change storage providers for the file storage types of phpBB. Choose local or remote providers to store files added to or created by phpBB.', + 'STORAGE_SELECT' => 'Select storage', + 'STORAGE_SELECT_DESC' => 'Select a storage from the list.', + 'STORAGE_NAME' => 'Storage name', + 'STORAGE_NUM_FILES' => 'Number of files', + 'STORAGE_SIZE' => 'Size', + 'STORAGE_FREE' => 'Available space', + 'STORAGE_UNKNOWN' => 'Unknown', + 'STORAGE_REMOVE_OLD_FILES' => 'Remove old files', + 'STORAGE_REMOVE_OLD_FILES_EXPLAIN' => 'Remove old files after they are copied to the new storage system.', // Storage names 'STORAGE_ATTACHMENT_TITLE' => 'Attachments storage', From 5e8bc99697e430def79d608a8d16b5c4b83c9437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Mon, 2 Jul 2018 03:34:14 +0200 Subject: [PATCH 02/34] [ticket/15699] Move line PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index f9e0e75ae0..8a027e4b07 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -177,10 +177,10 @@ class acp_storage fclose($stream); } - $this->db->sql_rowseek(0, $result); - if ($this->request->variable('remove_old', false)) { + $this->db->sql_rowseek(0, $result); + while ($row = $this->db->sql_fetchrow($result)) { $current_adapter->delete($row['file_path']); From 8f7e5fec0b29428f09a047b98bf8770f5d14fcf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 18 Jul 2018 14:00:52 +0200 Subject: [PATCH 03/34] [ticket/15699] Update acp_storage PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 8a027e4b07..50d18c251e 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -91,14 +91,17 @@ class acp_storage */ $phpbb_dispatcher->trigger_event('core.acp_storage_load'); - $this->overview($id, $mode); + @ini_set('memory_limit', '128M'); + + switch ($mode) + { + case 'settings': + $this->settings($id, $mode); + break; + } } - /** - * @param string $id - * @param string $mode - */ - public function overview($id, $mode) + public function settings($id, $mode) { $form_key = 'acp_storage'; add_form_key($form_key); From 683c99086cde66c8994803178012f17cee6bd803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Tue, 24 Jul 2018 11:34:47 +0200 Subject: [PATCH 04/34] [ticket/15699] Move file between storages with progress bar PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 140 +++++++----- phpBB/includes/acp/acp_storage.php | 349 ++++++++++++++++++++++------- phpBB/language/en/acp/common.php | 2 + phpBB/language/en/acp/storage.php | 10 + 4 files changed, 366 insertions(+), 135 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 49535c1a74..999d5622ea 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -4,59 +4,83 @@

{{ lang('STORAGE_TITLE') }}

-

{{ 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 }}
+

{L_CONTINUE_EXPLAIN}

-{% if S_ERROR %} +
+
+ {L_SUBMIT} +   + + {S_FORM_TOKEN} +
+
+ + +

{{ 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 %} + {% endif %} -
+ - {% for storage in STORAGES %} -
- {{ lang('STORAGE_' ~ storage.get_name | upper ~ '_TITLE') }} -
-

{{ lang('STORAGE_SELECT_DESC') }}
-
- + {% for provider in PROVIDERS if provider.is_available %} - {% endif %} - {% endfor %} - -
-
-
+ {% endfor %} + + + +
- {% for provider in PROVIDERS %} - {% if provider.is_available %} + {% 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 %} @@ -93,25 +117,25 @@ {% endfor %}
- {% endif %} + {% endfor %} {% endfor %} - {% endfor %} -
-
-

{{ lang('STORAGE_REMOVE_OLD_FILES_EXPLAIN') }}
-
- -
-
-
+
+
+

{{ lang('STORAGE_REMOVE_OLD_FILES_EXPLAIN') }}
+
+ +
+
+
-
- {{ lang('SUBMIT') }} -   - - {{ S_FORM_TOKEN }} -
- +
+ {{ lang('SUBMIT') }} +   + + {{ S_FORM_TOKEN }} +
+ + {% include 'overall_footer.html' %} diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 50d18c251e..af7141dc92 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -24,11 +24,14 @@ class acp_storage /** @var \phpbb\config\config $config */ protected $config; + /** @var \phpbb\db_text $config_text */ + protected $config_text; + /** @var \db\driver\driver_interface $db */ protected $db; - /** @var \phpbb\language\language $lang */ - protected $lang; + /** @var \phpbb\path_helper $path_helper */ + protected $path_helper; /** @var \phpbb\request\request */ protected $request; @@ -60,6 +63,9 @@ class acp_storage /** @var string */ public $u_action; + /** @var mixed */ + protected $state; + /** * @param string $id * @param string $mode @@ -69,9 +75,10 @@ class acp_storage 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->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'); @@ -81,7 +88,7 @@ class acp_storage $this->phpbb_root_path = $phpbb_root_path; // Add necesary language files - $this->lang->add_lang(['acp/storage']); + $this->user->add_lang(['acp/storage']); /** * Add language strings @@ -112,14 +119,172 @@ class 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', false)) + { + if (!check_form_key($form_key) || !check_link_hash($this->request->variable('hash', ''), 'acp_storage')) + { + trigger_error($this->user->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); + } + + if ($this->request->variable('cancel', false)) + { + $action = ''; + $this->state = false; + $this->save_state(); + } + } + + if ($action) + { + switch ($action) + { + case 'progress_bar': + $this->display_progress_bar(); + break; + case 'update': + // Just continue + break; + default: + trigger_error('NO_ACTION', E_USER_ERROR); + break; + } + + if (!check_form_key($form_key) || !check_link_hash($this->request->variable('hash', ''), 'acp_storage')) + { + trigger_error($this->user->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); + } + + // TODO: If both providers are the same, and remove + // old files is checked, the files could be only moved + + // Copy files from the old to the new storage + $i = 0; + foreach ($this->state['storages'] as $storage_name => $storage_options) + { + // Skip storages that have already moved files + if ($this->state['storage_index'] > $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 > " . $this->state['file_index']; + $result = $this->db->sql_query($sql); + + $starttime = microtime(true); + 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->user->lang('STORAGE_UPDATE_REDIRECT', $this->user->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state['storages']))); + } + + $stream = $current_adapter->read_stream($row['file_path']); + $new_adapter->write_stream($row['file_path'], $stream); + fclose($stream); + + $this->state['file_index'] = $row['file_id']; // Set last uploaded file + } + + // Copied all files of a storage, increase storage index and reset file index + $this->state['storage_index']++; + $this->state['file_index'] = 0; + } + + if ($this->state['remove_old']) + { + $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++) + { + 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 > " . $this->state['file_index']; + $result = $this->db->sql_query($sql); + + $starttime = microtime(true); + 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->user->lang('STORAGE_UPDATE_REMOVE_REDIRECT', $this->user->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; + } + } + + // 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); + } + + $this->state = false; + $this->save_state(); + + //$phpbb_log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_STORAGE_UPDATE', false); // todo + trigger_error($this->user->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) + { + $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->user->lang('CONTINUE_UPDATING'), + 'L_CONTINUE_EXPLAIN' => $this->user->lang('CONTINUE_UPDATING_EXPLAIN'), + )); + + return; + } + + // 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')) + { + trigger_error($this->user->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); + } + $modified_storages = []; if (!check_form_key($form_key)) { - $messages[] = $this->lang->lang('FORM_INVALID'); + $messages[] = $this->user->lang('FORM_INVALID'); } foreach ($this->storage_collection as $storage) @@ -133,7 +298,7 @@ class acp_storage $modified = false; // Check if provider have been modified - if ($this->get_new_provider($storage_name) != $this->get_current_provider($storage_name)) + if ($this->request->variable([$storage_name, 'provider'], '') != $this->get_current_provider($storage_name)) { $modified = true; } @@ -143,7 +308,7 @@ class acp_storage { foreach (array_keys($options) as $definition) { - if ($this->get_new_definition($storage_name, $definition) != $this->get_current_definition($storage_name, $definition)) + if ($this->request->variable([$storage_name, $definition], '') != $this->get_current_definition($storage_name, $definition)) { $modified = true; break; @@ -163,37 +328,41 @@ class acp_storage { 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 + 'remove_old' => $this->request->variable('remove_old', false), + 'storage_index' => 0, + 'file_index' => 0, + 'remove_storage_index' => 0, + ]; + + // Save in the state the selected storages and their configuration foreach ($modified_storages as $storage_name) { - $current_adapter = $this->get_current_adapter($storage_name); - $new_adapter = $this->get_new_adapter($storage_name); + $this->state['storages'][$storage_name]['provider'] = $this->request->variable([$storage_name, 'provider'], ''); - $sql = 'SELECT file_path - FROM ' . STORAGE_TABLE . " - WHERE storage = '" . $storage_name . "'"; - $result = $this->db->sql_query($sql); + $options = $this->get_provider_options($this->request->variable([$storage_name, 'provider'], '')); - while ($row = $this->db->sql_fetchrow($result)) + foreach (array_keys($options) as $definition) { - $stream = $current_adapter->read_stream($row['file_path']); - $new_adapter->write_stream($row['file_path'], $stream); - fclose($stream); + $this->state['storages'][$storage_name]['config'][$definition] = $this->request->variable([$storage_name, $definition], ''); } - - if ($this->request->variable('remove_old', false)) - { - $this->db->sql_rowseek(0, $result); - - while ($row = $this->db->sql_fetchrow($result)) - { - $current_adapter->delete($row['file_path']); - } - } - - $this->update_storage_config($storage_name); } - trigger_error($this->lang->lang('STORAGE_UPDATE_SUCCESSFUL') . adm_back_link($this->u_action), E_USER_NOTICE); + $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->user->lang('START_UPDATING'), + 'L_CONTINUE_EXPLAIN' => $this->user->lang('START_UPDATING_EXPLAIN'), + )); + + return; } else { @@ -202,9 +371,10 @@ class acp_storage } // If there is no changes - trigger_error($this->lang->lang('STORAGE_NO_CHANGES') . adm_back_link($this->u_action), E_USER_WARNING); + trigger_error($this->user->lang('STORAGE_NO_CHANGES') . adm_back_link($this->u_action), E_USER_WARNING); } + // Top table with stats of each storage $storage_stats = []; foreach ($this->storage_collection as $storage) { @@ -219,11 +389,11 @@ class acp_storage } catch (\phpbb\storage\exception\exception $e) { - $free_space = $this->lang->lang('STORAGE_UNKNOWN'); + $free_space = $this->user->lang('STORAGE_UNKNOWN'); } $storage_stats[] = [ - 'name' => $this->lang->lang('STORAGE_' . strtoupper($storage->get_name()) . '_TITLE'), + 'name' => $this->user->lang('STORAGE_' . strtoupper($storage->get_name()) . '_TITLE'), 'files' => $storage->get_num_files(), 'size' => get_formatted_filesize($storage->get_size()), 'free_space' => $free_space, @@ -231,15 +401,63 @@ class acp_storage } $this->template->assign_vars([ - 'STORAGES' => $this->storage_collection, - 'STORAGE_STATS' => $storage_stats, - 'PROVIDERS' => $this->provider_collection, + 'STORAGES' => $this->storage_collection, + 'STORAGE_STATS' => $storage_stats, + 'PROVIDERS' => $this->provider_collection, - 'ERROR_MSG' => implode('
', $messages), - 'S_ERROR' => !empty($messages), + 'ERROR_MSG' => implode('
', $messages), + 'S_ERROR' => !empty($messages), + + 'U_ACTION' => $this->u_action . '&hash=' . generate_link_hash('acp_storage'), ]); } + protected function display_progress_bar() + { + adm_page_header($this->user->lang('STORAGE_UPDATE_IN_PROGRESS')); + $this->template->set_filenames(array( + 'body' => 'progress_bar.html') + ); + $this->template->assign_vars(array( + 'L_PROGRESS' => $this->user->lang('STORAGE_UPDATE_IN_PROGRESS'), + 'L_PROGRESS_EXPLAIN' => $this->user->lang('STORAGE_UPDATE_IN_PROGRESS_EXPLAIN')) + ); + adm_page_footer(); + } + + function close_popup_js() + { + return "\n"; + } + + protected function save_state() + { + $state = $this->state; + + if ($state == false) + { + $state = []; + } + + $this->config_text->set('storage_update_state', json_encode($state)); + } + + protected function load_state() + { + $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 * @@ -251,17 +469,6 @@ class acp_storage return $this->config['storage\\' . $storage_name . '\\provider']; } - /** - * Get the new provider from the request - * - * @param string $storage_name Storage name - * @return string The new provider - */ - protected function get_new_provider($storage_name) - { - return $this->request->variable([$storage_name, 'provider'], ''); - } - /** * Get adapter definitions from a provider * @@ -285,18 +492,6 @@ class acp_storage return $this->config['storage\\' . $storage_name . '\\config\\' . $definition]; } - /** - * Get the new value of the definition of a storage from the request - * - * @param string $storage_name Storage name - * @param string $definition Definition - * @return string Definition value - */ - protected function get_new_definition($storage_name, $definition) - { - return $this->request->variable([$storage_name, $definition], ''); - } - /** * Validates data * @@ -305,56 +500,56 @@ class acp_storage */ protected function validate_data($storage_name, &$messages) { - $storage_title = $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'); + $storage_title = $this->user->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'); // Check if provider exists try { - $new_provider = $this->provider_collection->get_by_class($this->get_new_provider($storage_name)); + $new_provider = $this->provider_collection->get_by_class($this->request->variable([$storage_name, 'provider'], '')); } catch (\Exception $e) { - $messages[] = $this->lang->lang('STORAGE_PROVIDER_NOT_EXISTS', $storage_title); + $messages[] = $this->user->lang('STORAGE_PROVIDER_NOT_EXISTS', $storage_title); return; } // Check if provider is available if (!$new_provider->is_available()) { - $messages[] = $this->lang->lang('STORAGE_PROVIDER_NOT_AVAILABLE', $storage_title); + $messages[] = $this->user->lang('STORAGE_PROVIDER_NOT_AVAILABLE', $storage_title); return; } // Check options - $new_options = $this->get_provider_options($this->get_new_provider($storage_name)); + $new_options = $this->get_provider_options($this->request->variable([$storage_name, 'provider'], '')); foreach ($new_options as $definition_key => $definition_value) { - $provider = $this->provider_collection->get_by_class($this->get_new_provider($storage_name)); - $definition_title = $this->lang->lang('STORAGE_ADAPTER_' . strtoupper($provider->get_name()) . '_OPTION_' . strtoupper($definition_key)); + $provider = $this->provider_collection->get_by_class($this->request->variable([$storage_name, 'provider'], '')); + $definition_title = $this->user->lang('STORAGE_ADAPTER_' . strtoupper($provider->get_name()) . '_OPTION_' . strtoupper($definition_key)); - $value = $this->get_new_definition($storage_name, $definition_key); + $value = $this->request->variable([$storage_name, $definition_key], ''); switch ($definition_value['type']) { case 'email': if (!filter_var($value, FILTER_VALIDATE_EMAIL)) { - $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT', $definition_title, $storage_title); + $messages[] = $this->user->lang('STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT', $definition_title, $storage_title); } case 'text': case 'password': $maxlength = isset($definition_value['maxlength']) ? $definition_value['maxlength'] : 255; if (strlen($value) > $maxlength) { - $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG', $definition_title, $storage_title); + $messages[] = $this->user->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG', $definition_title, $storage_title); } break; case 'radio': case 'select': if (!in_array($value, array_values($definition_value['options']))) { - $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_SELECT_NOT_AVAILABLE', $definition_title, $storage_title); + $messages[] = $this->user->lang('STORAGE_FORM_TYPE_SELECT_NOT_AVAILABLE', $definition_title, $storage_title); } break; } @@ -377,14 +572,14 @@ class acp_storage } // Update provider - $this->config->set('storage\\' . $storage_name . '\\provider', $this->get_new_provider($storage_name)); + $this->config->set('storage\\' . $storage_name . '\\provider', $this->state['storages'][$storage_name]['provider']); // Set new storage config - $new_options = $this->get_provider_options($this->get_new_provider($storage_name)); + $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->get_new_definition($storage_name, $definition)); + $this->config->set('storage\\' . $storage_name . '\\config\\' . $definition, $this->state['storages'][$storage_name]['config'][$definition]); } } @@ -434,7 +629,7 @@ class acp_storage protected function get_new_adapter($storage_name) { - $provider = $this->get_new_provider($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()); @@ -443,7 +638,7 @@ class acp_storage $options = []; foreach (array_keys($definitions) as $definition) { - $options[$definition] = $this->get_new_definition($storage_name, $definition); + $options[$definition] = $this->state['storages'][$storage_name]['config'][$definition]; } $adapter->configure($options); diff --git a/phpBB/language/en/acp/common.php b/phpBB/language/en/acp/common.php index bcbbe5d931..729888d509 100644 --- a/phpBB/language/en/acp/common.php +++ b/phpBB/language/en/acp/common.php @@ -768,6 +768,8 @@ $lang = array_merge($lang, array( 'LOG_STYLE_EDIT_DETAILS' => 'Edited style
» %s', 'LOG_STYLE_EXPORT' => 'Exported style
» %s', + 'LOG_STORAGE_UPDATE' => 'Storage updated', + 'LOG_UPDATE_DATABASE' => 'Updated Database from version %1$s to version %2$s', 'LOG_UPDATE_PHPBB' => 'Updated phpBB from version %1$s to version %2$s', diff --git a/phpBB/language/en/acp/storage.php b/phpBB/language/en/acp/storage.php index ae3c007c4a..7c921b46e2 100644 --- a/phpBB/language/en/acp/storage.php +++ b/phpBB/language/en/acp/storage.php @@ -50,6 +50,16 @@ $lang = array_merge($lang, array( 'STORAGE_UNKNOWN' => 'Unknown', 'STORAGE_REMOVE_OLD_FILES' => 'Remove old files', 'STORAGE_REMOVE_OLD_FILES_EXPLAIN' => 'Remove old files after they are copied to the new storage system.', + 'START_UPDATING' => 'Start update process', + 'START_UPDATING_EXPLAIN' => 'Start the storage update process', + 'CONTINUE_UPDATING' => 'Continue previous update process', + 'CONTINUE_UPDATING_EXPLAIN' => 'An update process has been started. In order to access the storage settings page you will have to complete it or cancel it.', + 'SORAGE_UPDATE_REDIRECT' => 'Files of %s (%d/%d) are being moved.
', + 'SORAGE_UPDATE_REMOVE_REDIRECT' => 'Files of old %s (%d/%d) are being removed.
', + + // Template progress bar + 'STORAGE_UPDATE_IN_PROGRESS' => 'Storage update in progress', + 'STORAGE_UPDATE_IN_PROGRESS_EXPLAIN' => 'Files are being moved between storages. This can take some minutes.', // Storage names 'STORAGE_ATTACHMENT_TITLE' => 'Attachments storage', From a2c7255b5b0d37455b4273e8f47330a1dcc78e36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sat, 28 Jul 2018 12:25:54 +0200 Subject: [PATCH 05/34] [ticket/15699] Use twig syntax PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 999d5622ea..031f453cdd 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -4,7 +4,7 @@

{{ lang('STORAGE_TITLE') }}

- +{% if S_CONTINUE_UPDATING %} -

{L_CONTINUE_EXPLAIN}

+

{{ lang('CONTINUE_EXPLAIN') }}

-
+
- {L_SUBMIT} -   - - {S_FORM_TOKEN} + {{ lang('SUBMIT }} +   + + {{ S_FORM_TOKEN }}
- - +{% else %}

{{ lang('STORAGE_TITLE_EXPLAIN') }}

@@ -136,6 +135,6 @@ {{ S_FORM_TOKEN }} - +{% endif %} {% include 'overall_footer.html' %} From 6a54b01f6d01686045aaf38ac33ca01ee279552c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Mon, 30 Jul 2018 14:42:46 +0200 Subject: [PATCH 06/34] [ticket/15699] Remove comment PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index af7141dc92..bddf1cf460 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -158,9 +158,6 @@ class acp_storage trigger_error($this->user->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); } - // TODO: If both providers are the same, and remove - // old files is checked, the files could be only moved - // Copy files from the old to the new storage $i = 0; foreach ($this->state['storages'] as $storage_name => $storage_options) From 9dc67e81ff92bb20242ebaaa6b2579c0f76540f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 1 Aug 2018 20:59:23 +0200 Subject: [PATCH 07/34] [ticket/15699] Fix twig syntax PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 031f453cdd..97069b0b57 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -23,7 +23,7 @@
- {{ lang('SUBMIT }} + {{ lang('SUBMIT') }}   {{ S_FORM_TOKEN }} @@ -91,7 +91,7 @@ {% set input_value = attribute(config, 'storage\\' ~ storage.get_name ~ '\\config\\' ~ name) %}
- + {% if lang_defined(description) %}
{{ lang(description) }} {% endif %} From 8cb5ea8e209e881b71b1eff2379adc703bb441f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 1 Aug 2018 21:04:55 +0200 Subject: [PATCH 08/34] [ticket/15699] Check if is resource before close PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index bddf1cf460..3eb3449854 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -189,7 +189,11 @@ class acp_storage $stream = $current_adapter->read_stream($row['file_path']); $new_adapter->write_stream($row['file_path'], $stream); - fclose($stream); + + if (is_resource($stream)) + { + fclose($stream); + } $this->state['file_index'] = $row['file_id']; // Set last uploaded file } From 9c51b3099a429652ae8fbf2ad78d8af01606e6f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 1 Aug 2018 21:24:45 +0200 Subject: [PATCH 09/34] [ticket/15699] Fix messages PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 8 +++++--- phpBB/language/en/acp/storage.php | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 3eb3449854..967780745e 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -153,7 +153,7 @@ class acp_storage break; } - if (!check_form_key($form_key) || !check_link_hash($this->request->variable('hash', ''), 'acp_storage')) + if (!check_link_hash($this->request->variable('hash', ''), 'acp_storage')) { trigger_error($this->user->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); } @@ -163,8 +163,9 @@ class acp_storage foreach ($this->state['storages'] as $storage_name => $storage_options) { // Skip storages that have already moved files - if ($this->state['storage_index'] > $i++) + if ($this->state['storage_index'] > $i) { + $i++; continue; } @@ -209,8 +210,9 @@ class acp_storage foreach ($this->state['storages'] as $storage_name => $storage_options) { // Skip storages that have already moved files - if ($this->state['remove_storage_index'] > $i++) + if ($this->state['remove_storage_index'] > $i) { + $i++; continue; } diff --git a/phpBB/language/en/acp/storage.php b/phpBB/language/en/acp/storage.php index 7c921b46e2..892aa42e72 100644 --- a/phpBB/language/en/acp/storage.php +++ b/phpBB/language/en/acp/storage.php @@ -54,8 +54,8 @@ $lang = array_merge($lang, array( 'START_UPDATING_EXPLAIN' => 'Start the storage update process', 'CONTINUE_UPDATING' => 'Continue previous update process', 'CONTINUE_UPDATING_EXPLAIN' => 'An update process has been started. In order to access the storage settings page you will have to complete it or cancel it.', - 'SORAGE_UPDATE_REDIRECT' => 'Files of %s (%d/%d) are being moved.
', - 'SORAGE_UPDATE_REMOVE_REDIRECT' => 'Files of old %s (%d/%d) are being removed.
', + 'STORAGE_UPDATE_REDIRECT' => 'Files of %s (%d/%d) are being moved.
', + 'STORAGE_UPDATE_REMOVE_REDIRECT' => 'Files of old %s (%d/%d) are being removed.
', // Template progress bar 'STORAGE_UPDATE_IN_PROGRESS' => 'Storage update in progress', From 0c71ff5eab070a67e2037ed712b965e4fd400991 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Fri, 3 Aug 2018 12:08:23 +0200 Subject: [PATCH 10/34] [ticket/15699] Log update PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 967780745e..5e2d4a2d4f 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -30,6 +30,9 @@ class acp_storage /** @var \db\driver\driver_interface $db */ protected $db; + /** @var \log\log_interface $log */ + protected $log; + /** @var \phpbb\path_helper $path_helper */ protected $path_helper; @@ -78,6 +81,7 @@ class acp_storage $this->config_text = $phpbb_container->get('config_text'); $this->db = $phpbb_container->get('dbal.conn'); $this->filesystem = $phpbb_container->get('filesystem'); + $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'); @@ -254,7 +258,7 @@ class acp_storage $this->state = false; $this->save_state(); - //$phpbb_log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_STORAGE_UPDATE', false); // todo + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_STORAGE_UPDATE', false); // todo trigger_error($this->user->lang('STORAGE_UPDATE_SUCCESSFUL') . adm_back_link($this->u_action) . $this->close_popup_js()); } From 9f641963d6cb158c3b3012a5048188eda92a1843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Fri, 3 Aug 2018 12:17:41 +0200 Subject: [PATCH 11/34] [ticket/15699] Fix docblocks PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 5e2d4a2d4f..0941551c3e 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -27,10 +27,10 @@ class acp_storage /** @var \phpbb\db_text $config_text */ protected $config_text; - /** @var \db\driver\driver_interface $db */ + /** @var \phpbb\db\driver\driver_interface $db */ protected $db; - /** @var \log\log_interface $log */ + /** @var \phpbb\log\log_interface $log */ protected $log; /** @var \phpbb\path_helper $path_helper */ From 6de2d44c8705cf6b741f3447708aa7833dcacb57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Mon, 20 Aug 2018 16:49:08 +0200 Subject: [PATCH 12/34] [ticket/15699] Make adapter to know the storage name PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 0941551c3e..78ea0f941d 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -630,6 +630,7 @@ class acp_storage } $adapter->configure($options); + $adapter->set_storage($storage_name); return $adapter; } @@ -649,6 +650,7 @@ class acp_storage } $adapter->configure($options); + $adapter->set_storage($storage_name); return $adapter; } From 3a25c0222b2d5ba097adc8b65b2907c697da8ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sat, 25 Aug 2018 17:54:18 +0200 Subject: [PATCH 13/34] [ticket/15699] Add option to udpate configuration only PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 6 +- phpBB/includes/acp/acp_storage.php | 110 +++++++++++++++-------------- phpBB/language/en/acp/storage.php | 6 +- 3 files changed, 65 insertions(+), 57 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 97069b0b57..43bb855bf3 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -121,9 +121,11 @@
-

{{ lang('STORAGE_REMOVE_OLD_FILES_EXPLAIN') }}
+
- + + +
diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 78ea0f941d..dfa35bb35e 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -162,65 +162,21 @@ class acp_storage trigger_error($this->user->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); } - // Copy files from the old to the new storage - $i = 0; - foreach ($this->state['storages'] as $storage_name => $storage_options) - { - // Skip storages that have already moved files - if ($this->state['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 > " . $this->state['file_index']; - $result = $this->db->sql_query($sql); - - $starttime = microtime(true); - 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->user->lang('STORAGE_UPDATE_REDIRECT', $this->user->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state['storages']))); - } - - $stream = $current_adapter->read_stream($row['file_path']); - $new_adapter->write_stream($row['file_path'], $stream); - - if (is_resource($stream)) - { - fclose($stream); - } - - $this->state['file_index'] = $row['file_id']; // Set last uploaded file - } - - // Copied all files of a storage, increase storage index and reset file index - $this->state['storage_index']++; - $this->state['file_index'] = 0; - } - - if ($this->state['remove_old']) + // If update_type is copy or move, copy files from the old to the new storage + if ($this->state['update_type'] >= 1) { $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) + if ($this->state['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 . " @@ -235,18 +191,66 @@ class acp_storage { $this->save_state(); meta_refresh(1, append_sid($this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'))); - trigger_error($this->user->lang('STORAGE_UPDATE_REMOVE_REDIRECT', $this->user->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state['storages']))); + trigger_error($this->user->lang('STORAGE_UPDATE_REDIRECT', $this->user->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state['storages']))); } - $current_adapter->delete($row['file_path']); + $stream = $current_adapter->read_stream($row['file_path']); + $new_adapter->write_stream($row['file_path'], $stream); + + if (is_resource($stream)) + { + fclose($stream); + } $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']++; + // Copied all files of a storage, increase storage index and reset file index + $this->state['storage_index']++; $this->state['file_index'] = 0; } + + // If update_type is move files, remove the old files + if ($this->state['update_type'] == 2) + { + $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 > " . $this->state['file_index']; + $result = $this->db->sql_query($sql); + + $starttime = microtime(true); + 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->user->lang('STORAGE_UPDATE_REMOVE_REDIRECT', $this->user->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; + } + } } // Here all files have been copied/moved, so save new configuration @@ -339,7 +343,7 @@ class acp_storage $this->state = [ // Save the value of the checkbox, to remove all files from the // old storage once they have been successfully moved - 'remove_old' => $this->request->variable('remove_old', false), + 'update_type' => $this->request->variable('update_type', 0), 'storage_index' => 0, 'file_index' => 0, 'remove_storage_index' => 0, diff --git a/phpBB/language/en/acp/storage.php b/phpBB/language/en/acp/storage.php index 892aa42e72..1530af0522 100644 --- a/phpBB/language/en/acp/storage.php +++ b/phpBB/language/en/acp/storage.php @@ -48,8 +48,10 @@ $lang = array_merge($lang, array( 'STORAGE_SIZE' => 'Size', 'STORAGE_FREE' => 'Available space', 'STORAGE_UNKNOWN' => 'Unknown', - 'STORAGE_REMOVE_OLD_FILES' => 'Remove old files', - 'STORAGE_REMOVE_OLD_FILES_EXPLAIN' => 'Remove old files after they are copied to the new storage system.', + 'STORAGE_UPDATE_TYPE' => 'Update type', + 'STORAGE_UPDATE_TYPE_CONFIG' => 'Update configuration only', + 'STORAGE_UPDATE_TYPE_COPY' => 'Update configuration and copy files', + 'STORAGE_UPDATE_TYPE_MOVE' => 'Update configuration and move files', 'START_UPDATING' => 'Start update process', 'START_UPDATING_EXPLAIN' => 'Start the storage update process', 'CONTINUE_UPDATING' => 'Continue previous update process', From 2c9f9fa75efd3807d54245601a556806e2cce290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sat, 25 Aug 2018 18:01:58 +0200 Subject: [PATCH 14/34] [ticket/15699] Add information to logs PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 3 ++- phpBB/language/en/acp/common.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index dfa35bb35e..160d49c186 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -259,10 +259,11 @@ class acp_storage $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); // todo + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_STORAGE_UPDATE', false, $storages); trigger_error($this->user->lang('STORAGE_UPDATE_SUCCESSFUL') . adm_back_link($this->u_action) . $this->close_popup_js()); } diff --git a/phpBB/language/en/acp/common.php b/phpBB/language/en/acp/common.php index 729888d509..62c08f0226 100644 --- a/phpBB/language/en/acp/common.php +++ b/phpBB/language/en/acp/common.php @@ -768,7 +768,7 @@ $lang = array_merge($lang, array( 'LOG_STYLE_EDIT_DETAILS' => 'Edited style
» %s', 'LOG_STYLE_EXPORT' => 'Exported style
» %s', - 'LOG_STORAGE_UPDATE' => 'Storage updated', + 'LOG_STORAGE_UPDATE' => 'Storage updated
» %s', 'LOG_UPDATE_DATABASE' => 'Updated Database from version %1$s to version %2$s', 'LOG_UPDATE_PHPBB' => 'Updated phpBB from version %1$s to version %2$s', From 64e8bea62a5edef284534660bdd201dbf9fbb938 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sat, 25 Aug 2018 18:12:57 +0200 Subject: [PATCH 15/34] [ticket/15699] Lazy-load adapters PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 62 ++++++++++++++++++------------ 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 160d49c186..23af6131b9 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -622,41 +622,55 @@ class acp_storage protected function get_current_adapter($storage_name) { - $provider = $this->get_current_provider($storage_name); - $provider_class = $this->provider_collection->get_by_class($provider); + static $adapters = []; - $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) + if(!isset($adapters[$storage_name])) { - $options[$definition] = $this->get_current_definition($storage_name, $definition); + $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; } - $adapter->configure($options); - $adapter->set_storage($storage_name); - - return $adapter; + return $adapters[$storage_name]; } protected function get_new_adapter($storage_name) { - $provider = $this->state['storages'][$storage_name]['provider']; - $provider_class = $this->provider_collection->get_by_class($provider); + static $adapters = []; - $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) + if(!isset($adapters[$storage_name])) { - $options[$definition] = $this->state['storages'][$storage_name]['config'][$definition]; + $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; } - $adapter->configure($options); - $adapter->set_storage($storage_name); - - return $adapter; + return $adapters[$storage_name]; } } From d426f9e3204d2bccb95e724d68c26d68d47d160a Mon Sep 17 00:00:00 2001 From: rubencm Date: Fri, 14 Sep 2018 09:58:16 +0000 Subject: [PATCH 16/34] [ticket/15699] Use language PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 54 ++++++++++++++++-------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 23af6131b9..b25ada9ce0 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -30,6 +30,9 @@ class acp_storage /** @var \phpbb\db\driver\driver_interface $db */ protected $db; + /** @var \phpbb\language\language $log */ + protected $lang; + /** @var \phpbb\log\log_interface $log */ protected $log; @@ -81,6 +84,7 @@ class acp_storage $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'); @@ -92,7 +96,7 @@ class acp_storage $this->phpbb_root_path = $phpbb_root_path; // Add necesary language files - $this->user->add_lang(['acp/storage']); + $this->lang->add_lang(['acp/storage']); /** * Add language strings @@ -131,7 +135,7 @@ class acp_storage { if (!check_form_key($form_key) || !check_link_hash($this->request->variable('hash', ''), 'acp_storage')) { - trigger_error($this->user->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); + trigger_error($this->lang->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); } if ($this->request->variable('cancel', false)) @@ -159,7 +163,7 @@ class acp_storage if (!check_link_hash($this->request->variable('hash', ''), 'acp_storage')) { - trigger_error($this->user->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); + 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 @@ -191,7 +195,7 @@ class acp_storage { $this->save_state(); meta_refresh(1, append_sid($this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'))); - trigger_error($this->user->lang('STORAGE_UPDATE_REDIRECT', $this->user->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state['storages']))); + trigger_error($this->lang->lang('STORAGE_UPDATE_REDIRECT', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state['storages']))); } $stream = $current_adapter->read_stream($row['file_path']); @@ -238,7 +242,7 @@ class acp_storage { $this->save_state(); meta_refresh(1, append_sid($this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'))); - trigger_error($this->user->lang('STORAGE_UPDATE_REMOVE_REDIRECT', $this->user->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['storages']))); } $current_adapter->delete($row['file_path']); @@ -264,7 +268,7 @@ class acp_storage $this->save_state(); $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_STORAGE_UPDATE', false, $storages); - trigger_error($this->user->lang('STORAGE_UPDATE_SUCCESSFUL') . adm_back_link($this->u_action) . $this->close_popup_js()); + 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 @@ -274,8 +278,8 @@ class acp_storage '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->user->lang('CONTINUE_UPDATING'), - 'L_CONTINUE_EXPLAIN' => $this->user->lang('CONTINUE_UPDATING_EXPLAIN'), + 'L_CONTINUE' => $this->lang->lang('CONTINUE_UPDATING'), + 'L_CONTINUE_EXPLAIN' => $this->lang->lang('CONTINUE_UPDATING_EXPLAIN'), )); return; @@ -289,14 +293,14 @@ class acp_storage { if (!check_form_key($form_key) || !check_link_hash($this->request->variable('hash', ''), 'acp_storage')) { - trigger_error($this->user->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); + trigger_error($this->lang->lang('FORM_INVALID') . adm_back_link($this->u_action), E_USER_WARNING); } $modified_storages = []; if (!check_form_key($form_key)) { - $messages[] = $this->user->lang('FORM_INVALID'); + $messages[] = $this->lang->lang('FORM_INVALID'); } foreach ($this->storage_collection as $storage) @@ -370,8 +374,8 @@ class acp_storage '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->user->lang('START_UPDATING'), - 'L_CONTINUE_EXPLAIN' => $this->user->lang('START_UPDATING_EXPLAIN'), + 'L_CONTINUE' => $this->lang->lang('START_UPDATING'), + 'L_CONTINUE_EXPLAIN' => $this->lang->lang('START_UPDATING_EXPLAIN'), )); return; @@ -383,7 +387,7 @@ class acp_storage } // If there is no changes - trigger_error($this->user->lang('STORAGE_NO_CHANGES') . adm_back_link($this->u_action), E_USER_WARNING); + trigger_error($this->lang->lang('STORAGE_NO_CHANGES') . adm_back_link($this->u_action), E_USER_WARNING); } // Top table with stats of each storage @@ -401,11 +405,11 @@ class acp_storage } catch (\phpbb\storage\exception\exception $e) { - $free_space = $this->user->lang('STORAGE_UNKNOWN'); + $free_space = $this->lang->lang('STORAGE_UNKNOWN'); } $storage_stats[] = [ - 'name' => $this->user->lang('STORAGE_' . strtoupper($storage->get_name()) . '_TITLE'), + 'name' => $this->lang->lang('STORAGE_' . strtoupper($storage->get_name()) . '_TITLE'), 'files' => $storage->get_num_files(), 'size' => get_formatted_filesize($storage->get_size()), 'free_space' => $free_space, @@ -426,13 +430,13 @@ class acp_storage protected function display_progress_bar() { - adm_page_header($this->user->lang('STORAGE_UPDATE_IN_PROGRESS')); + adm_page_header($this->lang->lang('STORAGE_UPDATE_IN_PROGRESS')); $this->template->set_filenames(array( 'body' => 'progress_bar.html') ); $this->template->assign_vars(array( - 'L_PROGRESS' => $this->user->lang('STORAGE_UPDATE_IN_PROGRESS'), - 'L_PROGRESS_EXPLAIN' => $this->user->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(); } @@ -512,7 +516,7 @@ class acp_storage */ protected function validate_data($storage_name, &$messages) { - $storage_title = $this->user->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'); + $storage_title = $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'); // Check if provider exists try @@ -521,14 +525,14 @@ class acp_storage } catch (\Exception $e) { - $messages[] = $this->user->lang('STORAGE_PROVIDER_NOT_EXISTS', $storage_title); + $messages[] = $this->lang->lang('STORAGE_PROVIDER_NOT_EXISTS', $storage_title); return; } // Check if provider is available if (!$new_provider->is_available()) { - $messages[] = $this->user->lang('STORAGE_PROVIDER_NOT_AVAILABLE', $storage_title); + $messages[] = $this->lang->lang('STORAGE_PROVIDER_NOT_AVAILABLE', $storage_title); return; } @@ -538,7 +542,7 @@ class acp_storage foreach ($new_options as $definition_key => $definition_value) { $provider = $this->provider_collection->get_by_class($this->request->variable([$storage_name, 'provider'], '')); - $definition_title = $this->user->lang('STORAGE_ADAPTER_' . strtoupper($provider->get_name()) . '_OPTION_' . strtoupper($definition_key)); + $definition_title = $this->lang->lang('STORAGE_ADAPTER_' . strtoupper($provider->get_name()) . '_OPTION_' . strtoupper($definition_key)); $value = $this->request->variable([$storage_name, $definition_key], ''); @@ -547,21 +551,21 @@ class acp_storage case 'email': if (!filter_var($value, FILTER_VALIDATE_EMAIL)) { - $messages[] = $this->user->lang('STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT', $definition_title, $storage_title); + $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT', $definition_title, $storage_title); } case 'text': case 'password': $maxlength = isset($definition_value['maxlength']) ? $definition_value['maxlength'] : 255; if (strlen($value) > $maxlength) { - $messages[] = $this->user->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG', $definition_title, $storage_title); + $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG', $definition_title, $storage_title); } break; case 'radio': case 'select': if (!in_array($value, array_values($definition_value['options']))) { - $messages[] = $this->user->lang('STORAGE_FORM_TYPE_SELECT_NOT_AVAILABLE', $definition_title, $storage_title); + $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_SELECT_NOT_AVAILABLE', $definition_title, $storage_title); } break; } From b260e9a6f200336ada12d940e614fee0b4abc9a9 Mon Sep 17 00:00:00 2001 From: rubencm Date: Fri, 14 Sep 2018 12:19:35 +0000 Subject: [PATCH 17/34] [ticket/15699] Fix code style PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index b25ada9ce0..c38dac5773 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -628,7 +628,7 @@ class acp_storage { static $adapters = []; - if(!isset($adapters[$storage_name])) + if (!isset($adapters[$storage_name])) { $provider = $this->get_current_provider($storage_name); $provider_class = $this->provider_collection->get_by_class($provider); @@ -655,7 +655,7 @@ class acp_storage { static $adapters = []; - if(!isset($adapters[$storage_name])) + if (!isset($adapters[$storage_name])) { $provider = $this->state['storages'][$storage_name]['provider']; $provider_class = $this->provider_collection->get_by_class($provider); From b10409cb8a6379d14f1efae227b5856040b4b540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Mon, 1 Oct 2018 21:52:26 +0200 Subject: [PATCH 18/34] [ticket/15699] Update comments PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index c38dac5773..a2350b5916 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -24,7 +24,7 @@ class acp_storage /** @var \phpbb\config\config $config */ protected $config; - /** @var \phpbb\db_text $config_text */ + /** @var \phpbb\config\db_text $config_text */ protected $config_text; /** @var \phpbb\db\driver\driver_interface $db */ From 958a43cb872f7c80c55af7a0a1b8b0998c2db517 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Tue, 9 Oct 2018 20:47:26 +0000 Subject: [PATCH 19/34] [ticket/15699] Remove unused variable and switch PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index a2350b5916..e575079ff3 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -148,17 +148,13 @@ class acp_storage if ($action) { - switch ($action) + if ($action == 'progress_bar') { - case 'progress_bar': - $this->display_progress_bar(); - break; - case 'update': - // Just continue - break; - default: - trigger_error('NO_ACTION', E_USER_ERROR); - break; + $this->display_progress_bar(); + } + else if ($action != 'update') + { + trigger_error('NO_ACTION', E_USER_ERROR); } if (!check_link_hash($this->request->variable('hash', ''), 'acp_storage')) @@ -188,7 +184,6 @@ class acp_storage AND file_id > " . $this->state['file_index']; $result = $this->db->sql_query($sql); - $starttime = microtime(true); while ($row = $this->db->sql_fetchrow($result)) { if (!still_on_time()) @@ -235,7 +230,6 @@ class acp_storage AND file_id > " . $this->state['file_index']; $result = $this->db->sql_query($sql); - $starttime = microtime(true); while ($row = $this->db->sql_fetchrow($result)) { if (!still_on_time()) From 43b402a7dab7b016cac3bfb98a14885e470e2f72 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 7 Mar 2021 10:04:09 +0100 Subject: [PATCH 20/34] [ticket/15699] Update acp_storage to be compatible with twig 3 PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 82 +++++++++++++++++--------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 43bb855bf3..aa25b8feb3 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -69,53 +69,57 @@

{{ 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' %} - + {% elseif input_type == 'textarea' %} + + {% elseif input_type == 'radio' %} {% for option_name, option_value in options['options'] %} - + {{ lang(option_name) }} {% endfor %} - - {% endif %} -
-
- {% endfor %} -
+ {% elseif input_type == 'select' %} + + {% endif %} + + + {% endfor %} + + {% endif %} {% endfor %} {% endfor %} From a6cf020639949b8ce915a904aa850fd5db7b2fa1 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 7 Mar 2021 10:30:04 +0100 Subject: [PATCH 21/34] [ticket/15699] Adjust validate path for removed method PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index e575079ff3..2ec086a232 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -605,7 +605,7 @@ class acp_storage { if ($this->provider_collection->get_by_class($this->get_current_provider($storage_name))->get_name() == 'local' && isset($options['path'])) { - $path = $this->request->is_set_post('submit') ? $this->get_new_definition($storage_name, 'path') : $this->get_current_definition($storage_name, 'path'); + $path = $this->request->is_set_post('submit') ? $this->request->variable([$storage_name, 'path'], '') : $this->get_current_definition($storage_name, 'path'); if (empty($path)) { From fba8990fa08f153158ee895e54dd033671f8e211 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 7 Mar 2021 10:39:12 +0100 Subject: [PATCH 22/34] [ticket/15699] Add type hints, add use statements, improve code readability PHPBB3-15699 --- phpBB/includes/acp/acp_storage.php | 97 ++++++++++++++++++++++-------- 1 file changed, 71 insertions(+), 26 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 2ec086a232..72de799725 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -11,6 +11,17 @@ * */ +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\template\template; +use phpbb\user; + /** * @ignore */ @@ -21,37 +32,40 @@ if (!defined('IN_PHPBB')) class acp_storage { - /** @var \phpbb\config\config $config */ + /** @var config $config */ protected $config; - /** @var \phpbb\config\db_text $config_text */ + /** @var config_text $config_text */ protected $config_text; - /** @var \phpbb\db\driver\driver_interface $db */ + /** @var driver_interface $db */ protected $db; - /** @var \phpbb\language\language $log */ + /** @var language $log */ protected $lang; - /** @var \phpbb\log\log_interface $log */ + /** @var log_interface $log */ protected $log; - /** @var \phpbb\path_helper $path_helper */ + /** @var path_helper $path_helper */ protected $path_helper; - /** @var \phpbb\request\request */ + /** @var request */ protected $request; - /** @var \phpbb\template\template */ + /** @var template */ protected $template; - /** @var \phpbb\di\service_collection */ + /** @var user */ + protected $user; + + /** @var service_collection */ protected $adapter_collection; - /** @var \phpbb\di\service_collection */ + /** @var service_collection */ protected $provider_collection; - /** @var \phpbb\di\service_collection */ + /** @var service_collection */ protected $storage_collection; /** @var \phpbb\filesystem\filesystem */ @@ -422,7 +436,10 @@ class acp_storage ]); } - protected function display_progress_bar() + /** + * Display progress bar + */ + protected function display_progress_bar() : void { adm_page_header($this->lang->lang('STORAGE_UPDATE_IN_PROGRESS')); $this->template->set_filenames(array( @@ -435,7 +452,12 @@ class acp_storage adm_page_footer(); } - function close_popup_js() + /** + * Get JS code for closing popup + * + * @return string Popup JS code + */ + function close_popup_js() : string { return "\n"; } - protected function save_state() + /** + * Save state of storage update + */ + protected function save_state() : void { $state = $this->state; @@ -456,7 +481,10 @@ class acp_storage $this->config_text->set('storage_update_state', json_encode($state)); } - protected function load_state() + /** + * Load state of storage update + */ + protected function load_state() : void { $state = json_decode($this->config_text->get('storage_update_state'), true); @@ -474,7 +502,7 @@ class acp_storage * @param string $storage_name Storage name * @return string The current provider */ - protected function get_current_provider($storage_name) + protected function get_current_provider(string $storage_name) : string { return $this->config['storage\\' . $storage_name . '\\provider']; } @@ -485,7 +513,7 @@ class acp_storage * @param string $provider Provider class * @return array Adapter definitions */ - protected function get_provider_options($provider) + protected function get_provider_options(string $provider) : array { return $this->provider_collection->get_by_class($provider)->get_options(); } @@ -497,7 +525,7 @@ class acp_storage * @param string $definition Definition * @return string Definition value */ - protected function get_current_definition($storage_name, $definition) + protected function get_current_definition(string $storage_name, string $definition) : string { return $this->config['storage\\' . $storage_name . '\\config\\' . $definition]; } @@ -508,7 +536,7 @@ class acp_storage * @param string $storage_name Storage name * @param array $messages Reference to messages array */ - protected function validate_data($storage_name, &$messages) + protected function validate_data(string $storage_name, array &$messages) { $storage_title = $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'); @@ -547,6 +575,8 @@ class acp_storage { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT', $definition_title, $storage_title); } + // no break + case 'text': case 'password': $maxlength = isset($definition_value['maxlength']) ? $definition_value['maxlength'] : 255; @@ -554,14 +584,15 @@ class acp_storage { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG', $definition_title, $storage_title); } - break; + break; + case 'radio': case 'select': if (!in_array($value, array_values($definition_value['options']))) { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_SELECT_NOT_AVAILABLE', $definition_title, $storage_title); } - break; + break; } } } @@ -571,7 +602,7 @@ class acp_storage * * @param string $storage_name Storage name */ - protected function update_storage_config($storage_name) + protected function update_storage_config(string $storage_name) : void { $current_options = $this->get_provider_options($this->get_current_provider($storage_name)); @@ -598,10 +629,10 @@ class acp_storage * * @param string $storage_name Storage name * @param array $options Storage provider configuration keys - * @param array $messages Reference to error messages array + * @param array $messages Error messages array * @return void */ - protected function validate_path($storage_name, $options, &$messages) + protected function validate_path(string $storage_name, array $options, array &$messages) : void { if ($this->provider_collection->get_by_class($this->get_current_provider($storage_name))->get_name() == 'local' && isset($options['path'])) { @@ -618,7 +649,14 @@ class acp_storage } } - protected function get_current_adapter($storage_name) + /** + * 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 = []; @@ -645,7 +683,14 @@ class acp_storage return $adapters[$storage_name]; } - protected function get_new_adapter($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 = []; From 7b0fc16e640b081229efca45c028a32bd591a5f8 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sun, 7 Mar 2021 21:19:11 +0100 Subject: [PATCH 23/34] [ticket/15699] Update language string PHPBB3-15699 --- phpBB/language/en/acp/storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/language/en/acp/storage.php b/phpBB/language/en/acp/storage.php index 1530af0522..47ef2acdfc 100644 --- a/phpBB/language/en/acp/storage.php +++ b/phpBB/language/en/acp/storage.php @@ -56,7 +56,7 @@ $lang = array_merge($lang, array( 'START_UPDATING_EXPLAIN' => 'Start the storage update process', 'CONTINUE_UPDATING' => 'Continue previous update process', 'CONTINUE_UPDATING_EXPLAIN' => 'An update process has been started. In order to access the storage settings page you will have to complete it or cancel it.', - 'STORAGE_UPDATE_REDIRECT' => 'Files of %s (%d/%d) are being moved.
', + 'STORAGE_UPDATE_REDIRECT' => 'Files of %1$s (%2$d/%3$d) are being moved.
', 'STORAGE_UPDATE_REMOVE_REDIRECT' => 'Files of old %s (%d/%d) are being removed.
', // Template progress bar From 43fb6ebe7a9539442f41fb2c646e311d3373d2d4 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sun, 7 Mar 2021 21:19:32 +0100 Subject: [PATCH 24/34] [ticket/15699] Update language string PHPBB3-15699 --- phpBB/language/en/acp/storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/language/en/acp/storage.php b/phpBB/language/en/acp/storage.php index 47ef2acdfc..2ed3a1b2a5 100644 --- a/phpBB/language/en/acp/storage.php +++ b/phpBB/language/en/acp/storage.php @@ -57,7 +57,7 @@ $lang = array_merge($lang, array( 'CONTINUE_UPDATING' => 'Continue previous update process', 'CONTINUE_UPDATING_EXPLAIN' => 'An update process has been started. In order to access the storage settings page you will have to complete it or cancel it.', 'STORAGE_UPDATE_REDIRECT' => 'Files of %1$s (%2$d/%3$d) are being moved.
', - 'STORAGE_UPDATE_REMOVE_REDIRECT' => 'Files of old %s (%d/%d) are being removed.
', + 'STORAGE_UPDATE_REMOVE_REDIRECT' => 'Files of old %1$s (%2$d/%3$d) are being removed.
', // Template progress bar 'STORAGE_UPDATE_IN_PROGRESS' => 'Storage update in progress', From 4b46939258a4a474c63609b99984d6a2cb678d88 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sun, 21 Mar 2021 17:59:45 +0100 Subject: [PATCH 25/34] [ticket/15699] Apply suggested fixes PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 3 +-- phpBB/includes/acp/acp_storage.php | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index aa25b8feb3..59389f1e44 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -5,7 +5,7 @@

{{ 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); From 59a5163e2b02e0d82e281c5bd033637ed1594384 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sat, 23 Sep 2023 11:58:52 +0000 Subject: [PATCH 29/34] [ticket/15699] Fix code style PHPBB3-15699 Co-authored-by: Marc Alexander --- phpBB/includes/acp/acp_storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 996a0f641b..2e28ef9a50 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -247,7 +247,7 @@ class acp_storage $this->db->sql_freeresult($result); // 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_remove_storage_index($this->state_helper->remove_storage_index() + 1); $this->state_helper->set_file_index(0); } } From bb84af9a4860b51bec20f803af95fd2a7a74bf8e Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sat, 17 Feb 2024 19:19:09 +0100 Subject: [PATCH 30/34] [ticket/15699] Add progress bar and use template macros PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 31 ++--- .../style/acp_storage_update_inprogress.html | 38 +++--- .../style/acp_storage_update_progress.html | 20 +++ phpBB/includes/acp/acp_storage.php | 124 ++++++++++-------- phpBB/language/en/acp/storage.php | 10 +- phpBB/phpbb/storage/adapter_factory.php | 42 +++--- phpBB/phpbb/storage/helper.php | 19 +-- phpBB/phpbb/storage/provider/local.php | 5 +- phpBB/phpbb/storage/state_helper.php | 1 + phpBB/phpbb/storage/storage.php | 9 +- 10 files changed, 154 insertions(+), 145 deletions(-) create mode 100644 phpBB/adm/style/acp_storage_update_progress.html diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index bb9940554b..63f52a276e 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -61,34 +61,23 @@
{{ 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) %}
- + {% set title = 'STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_OPTION_' ~ name | upper %} + {% set description = 'STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_OPTION_' ~ name | upper ~ '_EXPLAIN' %} + {% 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' %} - + {% set input_name = storage.get_name ~ '[' ~ name ~ ']' %} + {% set input_value = attribute(config, 'storage\\' ~ storage.get_name ~ '\\config\\' ~ name) %} + + {% if options['type'] in ['text', 'password', 'email'] %} + {{ FormsBuildTemplate(options | merge({"name": input_name, "value": input_value})) }} + {% elseif options['type'] == 'textarea' %} + {{ FormsBuildTemplate(options | merge({"name": input_name, "content": input_value})) }} {% endif %}
diff --git a/phpBB/adm/style/acp_storage_update_inprogress.html b/phpBB/adm/style/acp_storage_update_inprogress.html index 9c4b9fd804..5f7614cb2c 100644 --- a/phpBB/adm/style/acp_storage_update_inprogress.html +++ b/phpBB/adm/style/acp_storage_update_inprogress.html @@ -4,27 +4,27 @@

{{ lang('STORAGE_TITLE') }}

- +

{{ lang('STORAGE_TITLE_EXPLAIN') }}

-

{{ lang('CONTINUE_EXPLAIN') }}

- -
-
+ +
{{ lang('SUBMIT') }} -   - + + {% if CONTINUE_PROGRESS %} +
+
+
+ {{ CONTINUE_PROGRESS.PERCENTAGE|number_format(2) ~ ' %' }} +
+ {% endif %} + +

+   + +

{{ S_FORM_TOKEN }}
diff --git a/phpBB/adm/style/acp_storage_update_progress.html b/phpBB/adm/style/acp_storage_update_progress.html new file mode 100644 index 0000000000..17ba67a037 --- /dev/null +++ b/phpBB/adm/style/acp_storage_update_progress.html @@ -0,0 +1,20 @@ +{% include 'overall_header.html' %} + + + +
+

{{ INDEXING_TITLE }}

+

+ {{ INDEXING_EXPLAIN }} + {% if INDEXING_PROGRESS_BAR %} +
+
+ {{ INDEXING_PROGRESS_BAR.PERCENTAGE|number_format(2) ~ ' %' }} + {% endif %} +

+
+ +{% include 'overall_footer.html' %} diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 2e28ef9a50..e11a778278 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -16,6 +16,7 @@ use phpbb\di\service_collection; use phpbb\language\language; use phpbb\log\log_interface; use phpbb\request\request; +use phpbb\storage\exception\storage_exception; use phpbb\storage\helper; use phpbb\storage\state_helper; use phpbb\storage\update_type; @@ -77,6 +78,9 @@ class acp_storage /** @var helper */ private $storage_helper; + /** @var string */ + private $storage_table; + /** * @param string $id * @param string $mode @@ -97,6 +101,7 @@ class acp_storage $this->phpbb_root_path = $phpbb_root_path; $this->state_helper = $phpbb_container->get('storage.state_helper'); $this->storage_helper = $phpbb_container->get('storage.helper'); + $this->storage_table = $phpbb_container->getParameter('tables.storage'); // Add necessary language files $this->lang->add_lang(['acp/storage']); @@ -128,10 +133,6 @@ class acp_storage { switch ($action) { - case 'progress_bar': - $this->display_progress_bar(); - break; - case 'update': $this->update_action(); break; @@ -151,7 +152,7 @@ class acp_storage // There is an updating in progress, show the form to continue or cancel if ($this->state_helper->is_action_in_progress()) { - $this->update_inprogress($id, $mode); + $this->update_inprogress(); } else { @@ -162,8 +163,6 @@ class acp_storage 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); @@ -182,7 +181,7 @@ class acp_storage } $sql = 'SELECT file_id, file_path - FROM ' . STORAGE_TABLE . " + FROM ' . $this->storage_table . " WHERE storage = '" . $this->db->sql_escape($storage_name) . "' AND file_id > " . $this->state_helper->file_index(); $result = $this->db->sql_query($sql); @@ -192,9 +191,8 @@ class acp_storage 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()))); + $this->display_progress_page(); + return; } // Copy file from old adapter to the new one @@ -223,7 +221,7 @@ class acp_storage } $sql = 'SELECT file_id, file_path - FROM ' . STORAGE_TABLE . " + FROM ' . $this->storage_table . " WHERE storage = '" . $this->db->sql_escape($storage_name) . "' AND file_id > " . $this->state_helper->file_index(); $result = $this->db->sql_query($sql); @@ -233,8 +231,8 @@ class acp_storage 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'))); - trigger_error($this->lang->lang('STORAGE_UPDATE_REMOVE_REDIRECT', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'), $i + 1, count($this->state_helper->storages()))); + $this->display_progress_page(); + return; } // remove file from old (current) adapter @@ -262,10 +260,10 @@ class acp_storage $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()); + trigger_error($this->lang->lang('STORAGE_UPDATE_SUCCESSFUL') . adm_back_link($this->u_action)); } - private function update_inprogress(string $id, string $mode): void + private function update_inprogress(): void { // Template from adm/style $this->tpl_name = 'acp_storage_update_inprogress'; @@ -274,10 +272,8 @@ class acp_storage $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'), + 'U_ACTION' => $this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'), + 'CONTINUE_PROGRESS' => $this->get_storage_update_progress(), )); } @@ -308,27 +304,14 @@ class acp_storage trigger_error(implode('
', $messages) . adm_back_link($this->u_action), E_USER_WARNING); } - // Start process and show form + // Start process and show progress if (!empty($modified_storages)) { // Create state $this->state_helper->init(update_type::from((int) $this->request->variable('update_type', update_type::CONFIG->value)), $modified_storages, $this->request); - // 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'), - )); - - // Template from adm/style - $this->tpl_name = 'acp_storage_update_inprogress'; - - // Set page title - $this->page_title = 'STORAGE_TITLE'; - + // Start displaying progress on first submit + $this->display_progress_page(); return; } @@ -413,7 +396,7 @@ class acp_storage { $free_space = get_formatted_filesize($storage->free_space()); } - catch (\phpbb\storage\exception\storage_exception $e) + catch (storage_exception $e) { $free_space = $this->lang->lang('STORAGE_UNKNOWN'); } @@ -432,33 +415,60 @@ class acp_storage } /** - * Display progress bar + * Display progress page */ - protected function display_progress_bar() : void + protected function display_progress_page() : void { + $u_action = append_sid($this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage')); + meta_refresh(1, $u_action); + adm_page_header($this->lang->lang('STORAGE_UPDATE_IN_PROGRESS')); - $this->template->set_filenames(array( - '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')) - ); + $this->template->set_filenames([ + 'body' => 'acp_storage_update_progress.html' + ]); + + $this->template->assign_vars([ + 'INDEXING_TITLE' => $this->lang->lang('STORAGE_UPDATE_IN_PROGRESS'), + 'INDEXING_EXPLAIN' => $this->lang->lang('STORAGE_UPDATE_IN_PROGRESS_EXPLAIN'), + 'INDEXING_PROGRESS_BAR' => $this->get_storage_update_progress(), + ]); adm_page_footer(); } - /** - * Get JS code for closing popup - * - * @return string Popup JS code - */ - function close_popup_js() : string + protected function get_storage_update_progress(): array { - return "\n"; + $file_index = $this->state_helper->file_index(); + $stage_is_copy = $this->state_helper->storage_index() < count($this->state_helper->storages()); + $storage_name = $this->state_helper->storages()[$stage_is_copy ? $this->state_helper->storage_index() : $this->state_helper->remove_storage_index()]; + + $sql = 'SELECT COUNT(file_id) as done_count + FROM ' . $this->storage_table . ' + WHERE file_id <= ' . $file_index . " + AND storage = '" . $this->db->sql_escape($storage_name) . "'"; + $result = $this->db->sql_query($sql); + $done_count = (int) $this->db->sql_fetchfield('done_count'); + $this->db->sql_freeresult($result); + + $sql = 'SELECT COUNT(file_id) as remain_count + FROM ' . $this->storage_table . " + WHERE file_id > ' . $file_index . ' + AND storage = '" . $this->db->sql_escape($storage_name) . "'"; + $result = $this->db->sql_query($sql); + $remain_count = (int) $this->db->sql_fetchfield('remain_count'); + $this->db->sql_freeresult($result); + + $total_count = $done_count + $remain_count; + $percent = $done_count / $total_count; + + $steps = $this->state_helper->storage_index() + $this->state_helper->remove_storage_index() + $percent; + $multiplier = $this->state_helper->update_type() === update_type::MOVE ? 2 : 1; + $steps_total = count($this->state_helper->storages()) * $multiplier; + + return [ + 'VALUE' => $steps, + 'TOTAL' => $steps_total, + 'PERCENTAGE' => $steps / $steps_total * 100, + ]; } /** diff --git a/phpBB/language/en/acp/storage.php b/phpBB/language/en/acp/storage.php index 2ed3a1b2a5..9f928ee58e 100644 --- a/phpBB/language/en/acp/storage.php +++ b/phpBB/language/en/acp/storage.php @@ -36,7 +36,7 @@ if (empty($lang) || !is_array($lang)) // equally where a string contains only two placeholders which are used to wrap text // in a url you again do not need to specify an order e.g., 'Click %sHERE%s' is fine -$lang = array_merge($lang, array( +$lang = array_merge($lang, [ // Template 'STORAGE_TITLE' => 'Storage Settings', @@ -52,12 +52,6 @@ $lang = array_merge($lang, array( 'STORAGE_UPDATE_TYPE_CONFIG' => 'Update configuration only', 'STORAGE_UPDATE_TYPE_COPY' => 'Update configuration and copy files', 'STORAGE_UPDATE_TYPE_MOVE' => 'Update configuration and move files', - 'START_UPDATING' => 'Start update process', - 'START_UPDATING_EXPLAIN' => 'Start the storage update process', - 'CONTINUE_UPDATING' => 'Continue previous update process', - 'CONTINUE_UPDATING_EXPLAIN' => 'An update process has been started. In order to access the storage settings page you will have to complete it or cancel it.', - 'STORAGE_UPDATE_REDIRECT' => 'Files of %1$s (%2$d/%3$d) are being moved.
', - 'STORAGE_UPDATE_REMOVE_REDIRECT' => 'Files of old %1$s (%2$d/%3$d) are being removed.
', // Template progress bar 'STORAGE_UPDATE_IN_PROGRESS' => 'Storage update in progress', @@ -83,4 +77,4 @@ $lang = array_merge($lang, array( 'STORAGE_PATH_NOT_EXISTS' => '“%1$s” path does not exist or is not writable.', 'STORAGE_PATH_NOT_SET' => '“%1$s” path is not set.', -)); +]); diff --git a/phpBB/phpbb/storage/adapter_factory.php b/phpBB/phpbb/storage/adapter_factory.php index b358328525..62e51f66ae 100644 --- a/phpBB/phpbb/storage/adapter_factory.php +++ b/phpBB/phpbb/storage/adapter_factory.php @@ -15,6 +15,7 @@ namespace phpbb\storage; use phpbb\config\config; use phpbb\di\service_collection; +use phpbb\storage\adapter\adapter_interface; use phpbb\storage\exception\storage_exception; class adapter_factory @@ -53,9 +54,24 @@ class adapter_factory * * @param string $storage_name * - * @return \phpbb\storage\adapter\adapter_interface + * @return adapter_interface */ - public function get($storage_name) + public function get(string $storage_name) + { + $provider_class = $this->config['storage\\' . $storage_name . '\\provider']; + $provider = $this->providers->get_by_class($provider_class); + + $options = []; + foreach (array_keys($provider->get_options()) as $definition) + { + /** @psalm-suppress InvalidArrayOffset */ + $options[$definition] = $this->config['storage\\' . $storage_name . '\\config\\' . $definition]; + } + + return $this->get_with_options($storage_name, $options); + } + + public function get_with_options(string $storage_name, array $options) { $provider_class = $this->config['storage\\' . $storage_name . '\\provider']; $provider = $this->providers->get_by_class($provider_class); @@ -66,28 +82,8 @@ class adapter_factory } $adapter = $this->adapters->get_by_class($provider->get_adapter_class()); - $adapter->configure($this->build_options($storage_name, $provider->get_options())); + $adapter->configure($options); return $adapter; } - - /** - * Obtains configuration for a given storage - * - * @param string $storage_name - * @param array $definitions - * - * @return array Returns storage configuration values - */ - public function build_options($storage_name, array $definitions) - { - $options = []; - - foreach (array_keys($definitions) as $definition) - { - $options[$definition] = $this->config['storage\\' . $storage_name . '\\config\\' . $definition]; - } - - return $options; - } } diff --git a/phpBB/phpbb/storage/helper.php b/phpBB/phpbb/storage/helper.php index 6a5e2cc28c..de75ab337c 100644 --- a/phpBB/phpbb/storage/helper.php +++ b/phpBB/phpbb/storage/helper.php @@ -45,12 +45,12 @@ class helper /** * Get adapter definitions from a provider * - * @param string $provider Provider class + * @param string $provider_class Provider class * @return array Adapter definitions */ - public function get_provider_options(string $provider) : array + public function get_provider_options(string $provider_class) : array { - return $this->provider_collection->get_by_class($provider)->get_options(); + return $this->provider_collection->get_by_class($provider_class)->get_options(); } /** @@ -108,21 +108,16 @@ class helper 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); + $provider_class = $this->state_helper->new_provider($storage_name); + $definitions = array_keys($this->get_provider_options($provider_class)); $options = []; - foreach (array_keys($definitions) as $definition) + foreach ($definitions as $definition) { $options[$definition] = $this->state_helper->new_definition_value($storage_name, $definition); } - $adapter->configure($options); - - $adapters[$storage_name] = $adapter; + $adapters[$storage_name] = $this->adapter_factory->get_with_options($storage_name, $options); } return $adapters[$storage_name]; diff --git a/phpBB/phpbb/storage/provider/local.php b/phpBB/phpbb/storage/provider/local.php index 3bec0c5ce5..c6338e0c31 100644 --- a/phpBB/phpbb/storage/provider/local.php +++ b/phpBB/phpbb/storage/provider/local.php @@ -37,7 +37,10 @@ class local implements provider_interface public function get_options() { return [ - 'path' => ['type' => 'text'], + 'path' => [ + 'tag' => 'input', + 'type' => 'text', + ], ]; } diff --git a/phpBB/phpbb/storage/state_helper.php b/phpBB/phpbb/storage/state_helper.php index cc9de6dff3..aa48c3377c 100644 --- a/phpBB/phpbb/storage/state_helper.php +++ b/phpBB/phpbb/storage/state_helper.php @@ -163,6 +163,7 @@ class state_helper foreach (array_keys($options) as $definition) { + /** @psalm-suppress InvalidArrayOffset */ $state['storages'][$storage_name]['config'][$definition] = $request->variable([$storage_name, $definition], ''); } } diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index af0c9f0c67..e99d046a54 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -15,6 +15,7 @@ namespace phpbb\storage; use phpbb\cache\driver\driver_interface as cache; use phpbb\db\driver\driver_interface as db; +use phpbb\storage\adapter\adapter_interface; use phpbb\storage\exception\storage_exception; /** @@ -23,7 +24,7 @@ use phpbb\storage\exception\storage_exception; class storage { /** - * @var \phpbb\storage\adapter\adapter_interface + * @var adapter_interface */ protected $adapter; @@ -39,7 +40,7 @@ class storage protected $cache; /** - * @var \phpbb\storage\adapter_factory + * @var adapter_factory */ protected $factory; @@ -58,7 +59,7 @@ class storage * * @param db $db * @param cache $cache - * @param \phpbb\storage\adapter_factory $factory + * @param adapter_factory $factory * @param string $storage_name * @param string $storage_table */ @@ -84,7 +85,7 @@ class storage /** * Returns an adapter instance * - * @return \phpbb\storage\adapter\adapter_interface + * @return adapter_interface */ protected function get_adapter() { From bf467f8ece9d58312b5674397f1863dc971908d5 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sat, 17 Feb 2024 20:15:39 +0100 Subject: [PATCH 31/34] [ticket/15699] Remove .phpunit.result.cache PHPBB3-15699 --- .gitignore | 1 + .phpunit.result.cache | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 .phpunit.result.cache diff --git a/.gitignore b/.gitignore index 69f93652be..6972ca1516 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,7 @@ # Excludes test / dev files /phpunit.xml +/.phpunit.result.cache /phpBB/composer.phar /tests/phpbb_unit_tests.sqlite* /tests/test_config*.php diff --git a/.phpunit.result.cache b/.phpunit.result.cache deleted file mode 100644 index 2de2a6daf1..0000000000 --- a/.phpunit.result.cache +++ /dev/null @@ -1 +0,0 @@ -{"version":1,"defects":[],"times":[]} \ No newline at end of file From 155b5168bee6e0f2ad23eddb247ce5671a1c61db Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sun, 18 Feb 2024 10:44:41 +0100 Subject: [PATCH 32/34] [ticket/15699] Improve code style PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 32 +++++++++++++-------------- phpBB/includes/acp/acp_storage.php | 6 ++--- phpBB/phpbb/storage/adapter/local.php | 2 -- phpBB/phpbb/storage/state_helper.php | 2 -- phpBB/phpbb/storage/storage.php | 8 +++---- 5 files changed, 23 insertions(+), 27 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 63f52a276e..39ca5bb012 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -16,34 +16,34 @@ - {% for storage in STORAGE_STATS %} - - {{ storage.name }} - {{ storage.files }} - {{ storage.size }} - {{ storage.free_space }} - - {% endfor %} + {% for storage in STORAGE_STATS %} + + {{ storage.name }} + {{ storage.files }} + {{ storage.size }} + {{ storage.free_space }} + + {% endfor %} {% if ERROR_MESSAGES is not empty %} -
-

{{ lang('WARNING') }}

- {% for ERROR_MESSAGE in ERROR_MESSAGES %} -

{{ ERROR_MESSAGE }}

- {% endfor %} -
+
+

{{ 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_' ~ storage.get_name | upper ~ '_TITLE') }}

{{ lang('STORAGE_SELECT_DESC') }}
- {% for provider in PROVIDERS %} {% if provider.is_available %}
diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index bd66516a3e..150d711672 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -458,7 +458,7 @@ class acp_storage $this->db->sql_freeresult($result); $total_count = $done_count + $remain_count; - $percent = $done_count / $total_count; + $percent = $total_count > 0 ? $done_count / $total_count : 0; $steps = $this->state_helper->storage_index() + $this->state_helper->remove_storage_index() + $percent; $multiplier = $this->state_helper->update_type() === update_type::MOVE ? 2 : 1; @@ -509,18 +509,15 @@ class acp_storage $value = $this->request->variable([$storage_name, $definition_key], ''); - switch ($definition_value['type']) + switch ($definition_value['tag']) { - case 'email': - if (!filter_var($value, FILTER_VALIDATE_EMAIL)) + case 'text': + if ($definition_value['type'] == 'email' && filter_var($value, FILTER_VALIDATE_EMAIL)) { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT', $definition_title, $storage_title); } - // no break - case 'text': - case 'password': - $maxlength = isset($definition_value['maxlength']) ? $definition_value['maxlength'] : 255; + $maxlength = isset($definition_value['max']) ? $definition_value['max'] : 255; if (strlen($value) > $maxlength) { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG', $definition_title, $storage_title); @@ -542,8 +539,34 @@ class acp_storage break; case 'radio': + $found = false; + foreach ($definition_value['buttons'] as $button) + { + if ($button['value'] == $value) + { + $found = true; + break; + } + } + + if (!$found) + { + $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_SELECT_NOT_AVAILABLE', $definition_title, $storage_title); + } + break; + case 'select': - if (!in_array($value, array_values($definition_value['options']))) + $found = false; + foreach ($definition_value['options'] as $option) + { + if ($option['value'] == $value) + { + $found = true; + break; + } + } + + if (!$found) { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_SELECT_NOT_AVAILABLE', $definition_title, $storage_title); } From 195fb59b4e5d75b7204d22c18c11049b59c0ecbd Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sat, 25 May 2024 16:35:44 +0200 Subject: [PATCH 34/34] [ticket/15699] Fixes code review PHPBB3-15699 --- phpBB/adm/style/acp_storage.html | 12 ++-- .../default/container/services_storage.yml | 4 +- phpBB/includes/acp/acp_storage.php | 52 +++++++++++--- phpBB/phpbb/storage/adapter_factory.php | 15 ++-- phpBB/phpbb/storage/helper.php | 65 +++++++++++++++--- phpBB/phpbb/storage/state_helper.php | 68 ++++++++++++++++++- 6 files changed, 181 insertions(+), 35 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 1aedc50ce1..86c9298828 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -41,7 +41,7 @@
{{ lang('STORAGE_' ~ storage.get_name | upper ~ '_TITLE') }}
-

{{ lang('STORAGE_SELECT_DESC') }}
+

{{ lang('STORAGE_SELECT_DESC') }}
{{ lang('STORAGE_UPDATE_TYPE_CONFIG') }} diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index 401357861d..496c30de5d 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -120,7 +120,7 @@ services: class: phpbb\storage\helper arguments: - '@config' - - '@storage.provider_collection' - - '@storage.adapter_collection' - '@storage.adapter.factory' - '@storage.state_helper' + - '@storage.provider_collection' + - '@storage.adapter_collection' diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 150d711672..2adf833562 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -123,6 +123,8 @@ class acp_storage } /** + * Method to route the request to the correct page + * * @param string $id * @param string $mode */ @@ -156,11 +158,16 @@ class acp_storage } else { - $this->settings_form($id, $mode); + $this->settings_form(); } } } + /** + * Page to update storage settings and move files + * + * @return void + */ private function update_action(): void { if (!check_link_hash($this->request->variable('hash', ''), 'acp_storage')) @@ -263,6 +270,11 @@ class acp_storage trigger_error($this->lang->lang('STORAGE_UPDATE_SUCCESSFUL') . adm_back_link($this->u_action)); } + /** + * Page that show a form with the progress bar, and a button to continue or cancel + * + * @return void + */ private function update_inprogress(): void { // Template from adm/style @@ -271,13 +283,18 @@ class acp_storage // Set page title $this->page_title = 'STORAGE_TITLE'; - $this->template->assign_vars(array( + $this->template->assign_vars([ 'U_ACTION' => $this->u_action . '&action=update&hash=' . generate_link_hash('acp_storage'), 'CONTINUE_PROGRESS' => $this->get_storage_update_progress(), - )); + ]); } - private function settings_form(string $id, string $mode): void + /** + * Main settings page, shows a form with all the storages and their configuration options + * + * @return void + */ + private function settings_form(): void { $form_key = 'acp_storage'; add_form_key($form_key); @@ -348,6 +365,12 @@ class acp_storage ]); } + /** + * When submit the settings form, check which storages have been modified + * to update only those. + * + * @return array + */ private function get_modified_storages(): array { $modified_storages = []; @@ -386,7 +409,12 @@ class acp_storage return $modified_storages; } - protected function storage_stats() + /** + * Fill template variables to show storage stats in settings page + * + * @return void + */ + protected function storage_stats(): void { // Top table with stats of each storage $storage_stats = []; @@ -435,6 +463,11 @@ class acp_storage adm_page_footer(); } + /** + * Get storage update progress to show progress bar + * + * @return array + */ protected function get_storage_update_progress(): array { $file_index = $this->state_helper->file_index(); @@ -477,7 +510,7 @@ class acp_storage * @param string $storage_name Storage name * @param array $messages Reference to messages array */ - protected function validate_data(string $storage_name, array &$messages) + protected function validate_data(string $storage_name, array &$messages): void { $storage_title = $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE'); @@ -499,6 +532,8 @@ class acp_storage return; } + $this->validate_path($storage_name, $messages); + // Check options $new_options = $this->storage_helper->get_provider_options($this->request->variable([$storage_name, 'provider'], '')); @@ -517,7 +552,7 @@ class acp_storage $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT', $definition_title, $storage_title); } - $maxlength = isset($definition_value['max']) ? $definition_value['max'] : 255; + $maxlength = $definition_value['max'] ?? 255; if (strlen($value) > $maxlength) { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG', $definition_title, $storage_title); @@ -589,7 +624,7 @@ class acp_storage if ($this->provider_collection->get_by_class($current_provider)->get_name() == 'local' && isset($options['path'])) { - $path = $this->storage_helper->get_current_definition($storage_name, 'path'); + $path = $this->request->is_set_post('submit') ? $this->request->variable([$storage_name, 'path'], '') : $this->storage_helper->get_current_definition($storage_name, 'path'); if (empty($path)) { @@ -602,5 +637,4 @@ class acp_storage } } - } diff --git a/phpBB/phpbb/storage/adapter_factory.php b/phpBB/phpbb/storage/adapter_factory.php index 62e51f66ae..61ea879482 100644 --- a/phpBB/phpbb/storage/adapter_factory.php +++ b/phpBB/phpbb/storage/adapter_factory.php @@ -15,7 +15,6 @@ namespace phpbb\storage; use phpbb\config\config; use phpbb\di\service_collection; -use phpbb\storage\adapter\adapter_interface; use phpbb\storage\exception\storage_exception; class adapter_factory @@ -54,9 +53,9 @@ class adapter_factory * * @param string $storage_name * - * @return adapter_interface + * @return mixed */ - public function get(string $storage_name) + public function get(string $storage_name): mixed { $provider_class = $this->config['storage\\' . $storage_name . '\\provider']; $provider = $this->providers->get_by_class($provider_class); @@ -71,7 +70,15 @@ class adapter_factory return $this->get_with_options($storage_name, $options); } - public function get_with_options(string $storage_name, array $options) + /** + * Obtains a configured adapters for a given storage with custom options + * + * @param string $storage_name + * @param array $options + * + * @return mixed + */ + public function get_with_options(string $storage_name, array $options): mixed { $provider_class = $this->config['storage\\' . $storage_name . '\\provider']; $provider = $this->providers->get_by_class($provider_class); diff --git a/phpBB/phpbb/storage/helper.php b/phpBB/phpbb/storage/helper.php index de75ab337c..66921b60a7 100644 --- a/phpBB/phpbb/storage/helper.php +++ b/phpBB/phpbb/storage/helper.php @@ -21,31 +21,41 @@ 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) + /** @var service_collection */ + protected $provider_collection; + + /** @var service_collection */ + protected $adapter_collection; + + /** + * Constructor + * + * @param config $config + * @param adapter_factory $adapter_factory + * @param state_helper $state_helper + * @param service_collection $provider_collection + * @param service_collection $adapter_collection + */ + public function __construct(config $config, adapter_factory $adapter_factory, state_helper $state_helper, service_collection $provider_collection, service_collection $adapter_collection) { $this->config = $config; - $this->provider_collection = $provider_collection; - $this->adapter_collection = $adapter_collection; $this->adapter_factory = $adapter_factory; $this->state_helper = $state_helper; + $this->provider_collection = $provider_collection; + $this->adapter_collection = $adapter_collection; } /** * Get adapter definitions from a provider * * @param string $provider_class Provider class + * * @return array Adapter definitions */ public function get_provider_options(string $provider_class) : array @@ -57,6 +67,7 @@ class helper * 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 @@ -69,6 +80,7 @@ class helper * * @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 @@ -102,7 +114,7 @@ class helper * * @return mixed Storage adapter instance */ - public function get_new_adapter(string $storage_name) + public function get_new_adapter(string $storage_name): mixed { static $adapters = []; @@ -123,6 +135,13 @@ class helper return $adapters[$storage_name]; } + /** + * Delete configuration options for a given storage + * + * @param string $storage_name + * + * @return void + */ public function delete_storage_options(string $storage_name): void { $provider = $this->get_current_provider($storage_name); @@ -134,16 +153,41 @@ class helper } } + /** + * Set a provider in configuration for a given storage + * + * @param string $storage_name + * @param string $provider + * + * @return void + */ public function set_storage_provider(string $storage_name, string $provider): void { $this->config->set('storage\\' . $storage_name . '\\provider', $provider); } + /** + * Set storage options in configuration for a given storage + * + * @param string $storage_name + * @param string $definition + * @param string $value + * + * @return void + */ public function set_storage_definition(string $storage_name, string $definition, string $value): void { $this->config->set('storage\\' . $storage_name . '\\config\\' . $definition, $value); } + /** + * Copy a file from the current adapter to the new adapter + * + * @param $storage_name + * @param $file + * + * @return void + */ public function copy_file_to_new_adapter($storage_name, $file): void { $current_adapter = $this->get_current_adapter($storage_name); @@ -166,7 +210,6 @@ class helper */ public function update_storage_config(string $storage_name) : void { - // Remove old storage config $this->delete_storage_options($storage_name); diff --git a/phpBB/phpbb/storage/state_helper.php b/phpBB/phpbb/storage/state_helper.php index 6105dc25da..82523f0aa0 100644 --- a/phpBB/phpbb/storage/state_helper.php +++ b/phpBB/phpbb/storage/state_helper.php @@ -31,6 +31,11 @@ class state_helper /** @var service_collection */ protected $provider_collection; + /** + * @param config $config + * @param db_text $config_text + * @param service_collection $provider_collection + */ public function __construct(config $config, db_text $config_text, service_collection $provider_collection) { $this->config = $config; @@ -48,6 +53,13 @@ class state_helper return !empty(json_decode($this->config_text->get('storage_update_state'), true)); } + /** + * Get new provider for the specified storage + * + * @param string $storage_name + * + * @return string + */ public function new_provider(string $storage_name): string { $state = $this->load_state(); @@ -55,6 +67,14 @@ class state_helper return $state['storages'][$storage_name]['provider']; } + /** + * Get new definition value for the specified storage + * + * @param string $storage_name + * @param string $definition + * + * @return string + */ public function new_definition_value(string $storage_name, string $definition): string { $state = $this->load_state(); @@ -62,6 +82,11 @@ class state_helper return $state['storages'][$storage_name]['config'][$definition]; } + /** + * Get the update type + * + * @return update_type + */ public function update_type(): update_type { $state = $this->load_state(); @@ -69,6 +94,11 @@ class state_helper return update_type::from($state['update_type']); } + /** + * Get the current storage index + * + * @return int + */ public function storage_index(): int { $state = $this->load_state(); @@ -76,6 +106,13 @@ class state_helper return $state['storage_index']; } + /** + * Update the storage index + * + * @param int $storage_index + * + * @return void + */ public function set_storage_index(int $storage_index): void { $state = $this->load_state(); @@ -85,6 +122,11 @@ class state_helper $this->save_state($state); } + /** + * Get the current remove storage index + * + * @return int + */ public function remove_storage_index(): int { $state = $this->load_state(); @@ -92,6 +134,13 @@ class state_helper return $state['remove_storage_index']; } + /** + * Update the remove storage index + * + * @param int $storage_index + * + * @return void + */ public function set_remove_storage_index(int $storage_index): void { $state = $this->load_state(); @@ -101,6 +150,11 @@ class state_helper $this->save_state($state); } + /** + * Get the file index + * + * @return int + */ public function file_index(): int { $state = $this->load_state(); @@ -108,6 +162,12 @@ class state_helper return $state['file_index']; } + /** + * Set the file index + * + * @param int $file_index + * @return void + */ public function set_file_index(int $file_index): void { $state = $this->load_state(); @@ -117,6 +177,11 @@ class state_helper $this->save_state($state); } + /** + * Get the storage names to be updated + * + * @return array + */ public function storages(): array { $state = $this->load_state(); @@ -178,7 +243,7 @@ class state_helper */ public function clear_state(): void { - $this->save_state([]); + $this->save_state(); } /** @@ -210,5 +275,4 @@ class state_helper { $this->config_text->set('storage_update_state', json_encode($state, JSON_THROW_ON_ERROR)); } - }