From f0df09a56e4324709466dc38ba5187029d08eea2 Mon Sep 17 00:00:00 2001 From: Marijus Dilys <106698165+MarijusDilys@users.noreply.github.com> Date: Thu, 16 Oct 2025 09:02:02 +0300 Subject: [PATCH] Revert "SL-329 optimize db" --- TASKS.md | 215 ------------------ src/Install/Installer.php | 23 +- .../SaferPayCardAliasRepository.php | 8 +- src/Repository/SaferPayOrderRepository.php | 11 +- upgrade/install-2.0.2.php | 104 +-------- 5 files changed, 17 insertions(+), 344 deletions(-) delete mode 100644 TASKS.md diff --git a/TASKS.md b/TASKS.md deleted file mode 100644 index 5edd4fd63..000000000 --- a/TASKS.md +++ /dev/null @@ -1,215 +0,0 @@ -# SaferPay Official Module - Improvement Tasks - -## Performance Optimizations - -### 1. Database Query Optimization -**Priority: High** | **Effort: Medium** | **Impact: High** - -- **Issue**: Multiple database queries without proper indexing and some inefficient queries -- **Location**: `src/Repository/` classes, especially `SaferPayCardAliasRepository.php` and `SaferPayOrderRepository.php` -- **Improvements**: - - Add database indexes for frequently queried columns (id_customer, payment_method, id_order, id_cart) - - Optimize queries in `getSavedValidCardsByUserIdAndPaymentMethod()` to use JOINs instead of multiple WHERE clauses - - Implement query result caching for static data like payment methods and configurations - - Add LIMIT clauses to queries that don't need all results - -### 2. Service Container Optimization -**Priority: Medium** | **Effort: Low** | **Impact: Medium** - -- **Issue**: Services are instantiated multiple times in the same request -- **Location**: Main module class `getService()` method -- **Improvements**: - - Implement service singleton pattern or dependency injection container caching - - Cache frequently used services like `LoggerInterface`, `Configuration`, and repository classes - - Reduce service instantiation in hooks and controllers - -### 3. Configuration Access Optimization -**Priority: Medium** | **Effort: Low** | **Impact: Medium** - -- **Issue**: Configuration values are fetched multiple times per request -- **Location**: Throughout the module, especially in payment processing -- **Improvements**: - - Cache configuration values in memory for the duration of the request - - Implement lazy loading for configuration values - - Group related configuration calls - -## Code Quality Improvements - -### 4. Type Declarations Enhancement -**Priority: Medium** | **Effort: Medium** | **Impact: Medium** - -- **Issue**: Missing return type declarations and inconsistent type hints -- **Location**: Service classes, repositories, and controllers -- **Improvements**: - - Add strict return type declarations to all public methods - - Implement proper type hints for array parameters and return values - - Add PHPDoc blocks with proper @param and @return annotations - - Use PHP 7.4+ typed properties where applicable - -### 5. Exception Handling Standardization -**Priority: High** | **Effort: Medium** | **Impact: High** - -- **Issue**: Inconsistent exception handling patterns across the module -- **Location**: Controllers and service classes -- **Improvements**: - - Standardize exception handling in all controllers using a common pattern - - Implement proper exception hierarchy with specific exception types - - Add consistent error logging with context information - - Improve error messages for better user experience - -### 6. Code Duplication Reduction -**Priority: Medium** | **Effort: Medium** | **Impact: Medium** - -- **Issue**: Repeated code patterns in controllers and services -- **Location**: Front controllers, especially validation and return controllers -- **Improvements**: - - Extract common validation logic into shared service classes - - Create base controller methods for common operations (redirects, error handling) - - Implement shared utilities for URL generation and parameter handling - - Consolidate similar database query patterns - -## Security Enhancements - -### 7. Input Validation Strengthening -**Priority: High** | **Effort: Low** | **Impact: High** - -- **Issue**: Some user inputs lack proper validation and sanitization -- **Location**: Front controllers and admin controllers -- **Improvements**: - - Add comprehensive input validation for all user-provided data - - Implement proper SQL injection prevention (already using pSQL but ensure consistency) - - Add CSRF token validation for admin operations - - Validate file uploads and external data sources - -### 8. Sensitive Data Protection -**Priority: High** | **Effort: Low** | **Impact: High** - -- **Issue**: Sensitive configuration data handling could be improved -- **Location**: Configuration storage and display -- **Improvements**: - - Implement proper password masking in admin forms - - Add encryption for sensitive configuration values - - Improve logging to avoid exposing sensitive data - - Implement proper session security - -## Maintainability Improvements - -### 9. Configuration Management Enhancement -**Priority: Medium** | **Effort: Medium** | **Impact: Medium** - -- **Issue**: Configuration handling is scattered and could be more organized -- **Location**: `SaferPayConfig.php` and admin settings controller -- **Improvements**: - - Group related configuration options logically - - Implement configuration validation on save - - Add configuration migration system for future updates - - Create configuration backup/restore functionality - -### 10. Logging System Enhancement -**Priority: Medium** | **Effort: Low** | **Impact: Medium** - -- **Issue**: Logging could be more structured and informative -- **Location**: `Logger.php` and throughout the module -- **Improvements**: - - Implement structured logging with consistent format - - Add log level configuration per operation type - - Implement log rotation and cleanup automation - - Add performance metrics logging for critical operations - -### 11. Error Message Localization -**Priority: Low** | **Effort: Medium** | **Impact: Medium** - -- **Issue**: Some error messages are not properly localized -- **Location**: Exception services and controllers -- **Improvements**: - - Ensure all user-facing messages are properly translated - - Add missing translation keys - - Implement fallback messages for missing translations - - Standardize message formatting - -## User Experience Improvements - -### 12. Admin Interface Optimization -**Priority: Low** | **Effort: Low** | **Impact: Medium** - -- **Issue**: Admin interface could be more user-friendly -- **Location**: Admin settings controller and templates -- **Improvements**: - - Add configuration validation feedback - - Implement auto-save for non-critical settings - - Add help tooltips and documentation links - - Improve form layout and organization - -### 13. Frontend Error Handling -**Priority: Medium** | **Effort: Low** | **Impact: Medium** - -- **Issue**: Frontend error messages could be more user-friendly -- **Location**: Front controllers and templates -- **Improvements**: - - Implement graceful error handling for payment failures - - Add retry mechanisms for temporary failures - - Improve error message clarity for end users - - Add progress indicators for long-running operations - -## Technical Debt Reduction - -### 14. Legacy Code Cleanup -**Priority: Low** | **Effort: Medium** | **Impact: Low** - -- **Issue**: Some legacy code patterns and unused code -- **Location**: Throughout the module -- **Improvements**: - - Remove unused imports and methods - - Update deprecated PrestaShop API usage - - Consolidate similar functionality - - Remove commented-out code - -### 15. Testing Infrastructure -**Priority: Low** | **Effort: High** | **Impact: High** - -- **Issue**: Limited test coverage for critical functionality -- **Location**: Missing comprehensive tests -- **Improvements**: - - Add unit tests for service classes - - Implement integration tests for payment flows - - Add automated testing for admin functionality - - Create test data fixtures for consistent testing - -## Implementation Priority - -### Phase 1 (High Priority - Security & Performance) -1. Database Query Optimization (#1) -2. Exception Handling Standardization (#5) -3. Input Validation Strengthening (#7) -4. Sensitive Data Protection (#8) - -### Phase 2 (Medium Priority - Quality & UX) -5. Type Declarations Enhancement (#4) -6. Code Duplication Reduction (#6) -7. Service Container Optimization (#2) -8. Configuration Access Optimization (#3) -9. Frontend Error Handling (#13) - -### Phase 3 (Low Priority - Polish & Maintenance) -10. Configuration Management Enhancement (#9) -11. Logging System Enhancement (#10) -12. Admin Interface Optimization (#12) -13. Error Message Localization (#11) -14. Legacy Code Cleanup (#14) -15. Testing Infrastructure (#15) - -## Notes - -- All improvements should maintain backward compatibility -- Changes should be thoroughly tested in both test and live environments -- Performance improvements should be measured and documented -- Security enhancements should be reviewed by security experts -- User experience improvements should be tested with real users - -## Estimated Total Effort -- **Phase 1**: 3-4 weeks -- **Phase 2**: 4-5 weeks -- **Phase 3**: 3-4 weeks -- **Total**: 10-13 weeks for complete implementation - -Each task includes specific file locations and implementation guidance to ensure efficient development. diff --git a/src/Install/Installer.php b/src/Install/Installer.php index 1cdfce80d..5523a22b3 100755 --- a/src/Install/Installer.php +++ b/src/Install/Installer.php @@ -223,9 +223,7 @@ private function installSaferPayOrderTable() `refunded` tinyint(1) DEFAULT 0, `canceled` tinyint(1) DEFAULT 0, `authorized` tinyint(1) DEFAULT 0, - `pending` tinyint(1) DEFAULT 0, - INDEX `idx_token` (`token`), - INDEX `idx_status` (`captured`, `refunded`, `canceled`, `authorized`, `pending`) + `pending` tinyint(1) DEFAULT 0 ) ENGINE=' . _MYSQL_ENGINE_ . ' DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci' ); } @@ -256,13 +254,7 @@ private function installSaferPayAssertTable() `card_number` VARCHAR(64) NOT NULL, `dcc_value` INTEGER(32) DEFAULT NULL, `dcc_currency_code` VARCHAR(64) DEFAULT NULL, - `authorized` tinyint(1) DEFAULT 0, - INDEX `idx_status` (`status`), - INDEX `idx_payment_id` (`payment_id`), - INDEX `idx_merchant_reference` (`merchant_reference`), - INDEX `idx_authorized` (`authorized`), - INDEX `idx_currency_code` (`currency_code`), - INDEX `idx_order_status` (`id_saferpay_order`, `status`) + `authorized` tinyint(1) DEFAULT 0 ) ENGINE=' . _MYSQL_ENGINE_ . ' DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci' ); } @@ -280,10 +272,7 @@ private function installSaferPayCardAlias() `payment_method` VARCHAR(64) NOT NULL, `valid_till` datetime NOT NULL, `date_add` datetime NOT NULL, - `date_upd` datetime NOT NULL, - INDEX `idx_success` (`success`), - INDEX `idx_customer_success` (`id_customer`, `success`), - INDEX `idx_customer_valid_till` (`id_customer`, `valid_till`) + `date_upd` datetime NOT NULL ) ENGINE=' . _MYSQL_ENGINE_ . ' DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci' ); } @@ -315,11 +304,7 @@ private function installOrderRefundTable() `transaction_id` VARCHAR(64) NOT NULL, `amount` INTEGER(20) NOT NULL, `currency` VARCHAR(64) NOT NULL, - `status` VARCHAR(64) NOT NULL, - INDEX `idx_status` (`status`), - INDEX `idx_currency` (`currency`), - INDEX `idx_order_status` (`id_order`, `status`), - INDEX `idx_saferpay_order_status` (`id_saferpay_order`, `status`) + `status` VARCHAR(64) NOT NULL ) ENGINE=' . _MYSQL_ENGINE_ . ' DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci' ); } diff --git a/src/Repository/SaferPayCardAliasRepository.php b/src/Repository/SaferPayCardAliasRepository.php index 70fb0da73..d1664313e 100755 --- a/src/Repository/SaferPayCardAliasRepository.php +++ b/src/Repository/SaferPayCardAliasRepository.php @@ -37,7 +37,7 @@ public function getSavedValidCardsByUserIdAndPaymentMethod($userId, $paymentMeth $query = new DbQuery(); $query->select('`id_saferpay_card_alias`, `card_number`'); $query->from('saferpay_card_alias'); - $query->where('id_customer = ' . (int) $userId); + $query->where('id_customer = "' . (int) $userId . '"'); $query->where('payment_method = "' . pSQL($paymentMethod) . '"'); $query->where('valid_till > "' . pSQL($currentDate) . '"'); @@ -49,7 +49,7 @@ public function getSavedCardAliasFromId($id) $query = new DbQuery(); $query->select('`alias_id`'); $query->from('saferpay_card_alias'); - $query->where('id_saferpay_card_alias = ' . (int) $id); + $query->where('id_saferpay_card_alias = "' . (int) $id . '"'); return Db::getInstance()->getValue($query); } @@ -59,7 +59,7 @@ public function getSavedCardIdByCustomerIdAndAliasId($customerId, $aliasId) $query = new DbQuery(); $query->select('`id_saferpay_card_alias`'); $query->from('saferpay_card_alias'); - $query->where('id_customer = ' . (int) $customerId); + $query->where('id_customer = "' . (int) $customerId . '"'); $query->where('alias_id = "' . pSQL($aliasId) . '"'); return Db::getInstance()->getValue($query); @@ -70,7 +70,7 @@ public function getSavedCardsByCustomerId($customerId) $query = new DbQuery(); $query->select('*'); $query->from('saferpay_card_alias'); - $query->where('id_customer = ' . (int) $customerId); + $query->where('id_customer = "' . (int) $customerId . '"'); return Db::getInstance()->executeS($query); } diff --git a/src/Repository/SaferPayOrderRepository.php b/src/Repository/SaferPayOrderRepository.php index b7284ab4a..921a401f4 100755 --- a/src/Repository/SaferPayOrderRepository.php +++ b/src/Repository/SaferPayOrderRepository.php @@ -49,7 +49,7 @@ public function getIdByOrderId($orderId) $query = new DbQuery(); $query->select('`id_saferpay_order`'); $query->from('saferpay_order'); - $query->where('id_order = ' . (int) $orderId); + $query->where('id_order = "' . (int) $orderId . '"'); $query->orderBy('`id_saferpay_order` DESC'); return Db::getInstance()->getValue($query); @@ -60,7 +60,7 @@ public function getIdByCartId($cartId) $query = new DbQuery(); $query->select('`id_saferpay_order`'); $query->from('saferpay_order'); - $query->where('id_cart = ' . (int) $cartId); + $query->where('id_cart = "' . (int) $cartId . '"'); $query->orderBy('`id_saferpay_order` DESC'); return Db::getInstance()->getValue($query); @@ -70,7 +70,7 @@ public function getAssertIdBySaferPayOrderId($saferPayOrderId) $query = new DbQuery(); $query->select('`id_saferpay_assert`'); $query->from('saferpay_assert'); - $query->where('id_saferPay_order = ' . (int) $saferPayOrderId); + $query->where('id_saferPay_order = "' . (int) $saferPayOrderId . '"'); $query->orderBy('id_saferpay_assert DESC'); return Db::getInstance()->getValue($query); @@ -86,8 +86,7 @@ public function getOrderRefunds($saferPayOrderId) $query = new DbQuery(); $query->select('*'); $query->from('saferpay_order_refund'); - $query->where('id_saferPay_order = ' . (int) $saferPayOrderId); - $query->orderBy('id_saferpay_order_refund DESC'); + $query->where('id_saferPay_order = "' . (int) $saferPayOrderId . '"'); return Db::getInstance()->executeS($query); } @@ -97,7 +96,7 @@ public function getPaymentBrandBySaferpayOrderId($saferpayOrderId) $query = new DbQuery(); $query->select('`brand`'); $query->from('saferpay_assert'); - $query->where('id_saferpay_order = ' . (int) $saferpayOrderId); + $query->where('id_saferpay_order = "' . (int) $saferpayOrderId . '"'); return Db::getInstance()->getValue($query); } diff --git a/upgrade/install-2.0.2.php b/upgrade/install-2.0.2.php index 24e3b5e40..e66a64d06 100644 --- a/upgrade/install-2.0.2.php +++ b/upgrade/install-2.0.2.php @@ -25,106 +25,10 @@ exit; } -function upgrade_module_2_0_2(SaferPayOfficial $module) +function upgrade_module_2_0_2() { - $db = Db::getInstance(); - $success = true; + Configuration::updateValue('SAFERPAY_SEND_ORDER_CONF_MAIL', 0); + Configuration::updateValue('SAFERPAY_GROUP_CARDS', 0); - // Add indexes for saferpay_order table - $orderIndexes = [ - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_id_order` (`id_order`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_id_cart` (`id_cart`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_id_customer` (`id_customer`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_transaction_id` (`transaction_id`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_status_flags` (`authorized`, `captured`, `pending`)", - ]; - - foreach ($orderIndexes as $indexSql) { - try { - $result = $db->execute($indexSql); - if (!$result) { - $error = $db->getMsgError(); - if (strpos($error, 'Duplicate key name') === false) { - $success = false; - PrestaShopLogger::addLog('SaferPay: Failed to add order index - ' . $error, 3, null, 'SaferPayOrder'); - } - } - } catch (Exception $e) { - PrestaShopLogger::addLog('SaferPay: Order index creation skipped - ' . $e->getMessage(), 1, null, 'SaferPayOrder'); - } - } - - // Add indexes for saferpay_card_alias table - $cardAliasIndexes = [ - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_id_customer` (`id_customer`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_payment_method` (`payment_method`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_customer_payment` (`id_customer`, `payment_method`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_valid_till` (`valid_till`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_alias_id` (`alias_id`)", - ]; - - foreach ($cardAliasIndexes as $indexSql) { - try { - $result = $db->execute($indexSql); - if (!$result) { - $error = $db->getMsgError(); - if (strpos($error, 'Duplicate key name') === false) { - $success = false; - PrestaShopLogger::addLog('SaferPay: Failed to add card alias index - ' . $error, 3, null, 'SaferPayCardAlias'); - } - } - } catch (Exception $e) { - PrestaShopLogger::addLog('SaferPay: Card alias index creation skipped - ' . $e->getMessage(), 1, null, 'SaferPayCardAlias'); - } - } - - // Add indexes for saferpay_assert table - $assertIndexes = [ - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_assert` ADD INDEX `idx_id_saferpay_order` (`id_saferpay_order`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_assert` ADD INDEX `idx_payment_method` (`payment_method`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_assert` ADD INDEX `idx_brand` (`brand`)", - ]; - - foreach ($assertIndexes as $indexSql) { - try { - $result = $db->execute($indexSql); - if (!$result) { - $error = $db->getMsgError(); - if (strpos($error, 'Duplicate key name') === false) { - $success = false; - PrestaShopLogger::addLog('SaferPay: Failed to add assert index - ' . $error, 3, null, 'SaferPayAssert'); - } - } - } catch (Exception $e) { - PrestaShopLogger::addLog('SaferPay: Assert index creation skipped - ' . $e->getMessage(), 1, null, 'SaferPayAssert'); - } - } - - // Add indexes for saferpay_order_refund table - $refundIndexes = [ - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order_refund` ADD INDEX `idx_id_saferpay_order` (`id_saferpay_order`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order_refund` ADD INDEX `idx_id_order` (`id_order`)", - "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order_refund` ADD INDEX `idx_transaction_id` (`transaction_id`)", - ]; - - foreach ($refundIndexes as $indexSql) { - try { - $result = $db->execute($indexSql); - if (!$result) { - $error = $db->getMsgError(); - if (strpos($error, 'Duplicate key name') === false) { - $success = false; - PrestaShopLogger::addLog('SaferPay: Failed to add refund index - ' . $error, 3, null, 'SaferPayOrderRefund'); - } - } - } catch (Exception $e) { - PrestaShopLogger::addLog('SaferPay: Refund index creation skipped - ' . $e->getMessage(), 1, null, 'SaferPayOrderRefund'); - } - } - - if ($success) { - PrestaShopLogger::addLog('SaferPay: Database indexes added successfully', 1, null, 'SaferPayOptimization'); - } - - return $success; + return true; }