Database Rollback not working M2.3.0 #21151

Stepa4man · 2019-02-12T12:51:36Z

Provided support MySQL views

Description (*)

DB backup didn't differentiate tables and views

Fixed Issues

magento-engcom/msi#1988 Database Rollback not working M2.3.0

Manual testing scenarios (*)

create any view in magento db
bin/magento setup:backup --db
bin/magento setup:rollback --db-file=db_backup_file.sql -n

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

magento-engcom-team · 2019-02-12T12:51:39Z

Hi @Stepa4man. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento-engcom-team give me test instance - deploy test instance based on PR changes
@magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

app/code/Magento/Backup/Model/ResourceModel/Table/GetListTables.php

osrecio · 2019-02-12T14:02:43Z

app/code/Magento/Backup/Model/Db.php

     * @param Helper|null $helper
     */
    public function __construct(
        \Magento\Backup\Model\ResourceModel\Db $resourceDb,
        \Magento\Framework\App\ResourceConnection $resource,
+        GetListTables $getListTables = null,


Should be added to end of the construct Backward Compatibility (Adding a constructor parameter)

osrecio · 2019-02-12T15:21:52Z

app/code/Magento/Backup/Model/ResourceModel/Table/GetListTables.php

+        return $this->resource->getConnection('backup')->fetchCol(
+"SHOW FULL TABLES WHERE `Table_type` = ?",
+            self::TABLE_TYPE
+            self::TABLE_TYPE


Duplicate constant... Delete one please

looks like github's suggested change doesn't work properly...

Is a BETA feature. Maybe sometimes doesn't work properly. Sorry

maghamed · 2019-02-26T23:53:08Z

app/code/Magento/Backup/Model/ResourceModel/View/GetViewsBackup.php

+    private function getShowCreateView($viewName)
+    {
+        $quotedViewName = $this->getConnection()->quoteIdentifier($viewName);
+        $query = 'SHOW CREATE VIEW ' . $quotedViewName;


creating View we have to use this syntax

SQL SECURITY INVOKER

the same we do in MSI - https://github.com/magento-engcom/msi/blob/2.3-develop/app/code/Magento/InventoryCatalog/Setup/Patch/Schema/CreateLegacyStockStatusView.php#L69

https://dev.mysql.com/doc/refman/5.6/en/stored-programs-security.html

For a stored routine or view, use SQL SECURITY INVOKER in the object definition when possible so that it can be used only by users with permissions appropriate for the operations performed by the object.

In opposite case, MySQL user working with Magento would not be possible to invoke the View after creation.
This is especially affect our Cloud deployment

@maghamed

If this VIEW was created with SQL SECURITY INVOKER declaration - SHOW CREATE VIEW will show SQL statement with this declaration also.

If no - then with SQL SECURITY DEFINER (in this example inventory_stock_2 is also VIEW and was created without SQL SECURITY INVOKER declaration)

So it's ok with the current implementation

Agree, we already use SQL SECURITY INVOKER at the time of View creation on MSI, so that backup with just pick this setting and at the time of DB restore we keep that setting

maghamed · 2019-02-26T23:54:36Z

app/code/Magento/Backup/Model/ResourceModel/Table/GetListTables.php

+ */
+class GetListTables
+{
+    const TABLE_TYPE = 'BASE TABLE';


use private constants to prevent polluting public contract with excessive obligations

maghamed · 2019-02-26T23:55:14Z

app/code/Magento/Backup/Model/ResourceModel/View/GetListViews.php

+ */
+class GetListViews
+{
+    const TABLE_TYPE = 'VIEW';


use private constant instead

maghamed

@Stepa4man could you please apply requested changes asap.

We really want to include this PR into Magento 2.3.1 which is being prepared and assembled literally now

magento-cicd2 · 2019-02-27T09:07:38Z

All committers have signed the CLA.

ghost · 2019-03-02T02:20:16Z

Hi @Stepa4man, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento-engcom-team added Component: Backup Release Line: 2.3 labels Feb 12, 2019

Stepa4man requested a review from maghamed February 12, 2019 12:51

Stepa4man mentioned this pull request Feb 12, 2019

Database Rollback not working M2.3.0 magento/inventory#1988

Closed

osrecio reviewed Feb 12, 2019

View reviewed changes

app/code/Magento/Backup/Model/ResourceModel/Table/GetListTables.php Outdated Show resolved Hide resolved

osrecio self-assigned this Feb 12, 2019

osrecio reviewed Feb 12, 2019

View reviewed changes

maghamed reviewed Feb 26, 2019

View reviewed changes

maghamed suggested changes Feb 26, 2019

View reviewed changes

maghamed added the Award: special achievement label Feb 27, 2019

maghamed added the Award: complex label Feb 27, 2019

magento-engcom-team added the Progress: accept label Feb 27, 2019

MSI-1988 issue fixed: Database Rollback not working M2.3

07e3cf5

Stepa4man added the Partner: TechDivision label Feb 28, 2019

#21151: Fixed static tests

b11652b

Stepa4man added the partners-contribution Pull Request is created by Magento Partner label Mar 1, 2019

magento-engcom-team merged commit b11652b into magento:2.3-develop Mar 2, 2019

magento-engcom-team pushed a commit that referenced this pull request Mar 2, 2019

ENGCOM-4392: Database Rollback not working M2.3.0 #21151

e969b40

magento-engcom-team added this to the Release: 2.3.2 milestone Mar 2, 2019

magento-engcom-team added the Award: bug fix label Mar 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Database Rollback not working M2.3.0 #21151

Database Rollback not working M2.3.0 #21151

Stepa4man commented Feb 12, 2019 •

edited

Loading

magento-engcom-team commented Feb 12, 2019

osrecio Feb 12, 2019 •

edited

Loading

osrecio Feb 12, 2019

Stepa4man Feb 12, 2019

osrecio Feb 12, 2019 •

edited

Loading

maghamed Feb 26, 2019 •

edited

Loading

Stepa4man Feb 27, 2019 •

edited

Loading

maghamed Feb 27, 2019

maghamed Feb 26, 2019

Stepa4man Feb 27, 2019

maghamed Feb 26, 2019

Stepa4man Feb 27, 2019

maghamed left a comment

magento-cicd2 commented Feb 27, 2019 •

edited

Loading

ghost commented Mar 2, 2019

Database Rollback not working M2.3.0 #21151

Database Rollback not working M2.3.0 #21151

Conversation

Stepa4man commented Feb 12, 2019 • edited Loading

Description (*)

Fixed Issues

Manual testing scenarios (*)

Contribution checklist (*)

magento-engcom-team commented Feb 12, 2019

osrecio Feb 12, 2019 • edited Loading

Choose a reason for hiding this comment

osrecio Feb 12, 2019

Choose a reason for hiding this comment

Stepa4man Feb 12, 2019

Choose a reason for hiding this comment

osrecio Feb 12, 2019 • edited Loading

Choose a reason for hiding this comment

maghamed Feb 26, 2019 • edited Loading

Choose a reason for hiding this comment

Stepa4man Feb 27, 2019 • edited Loading

Choose a reason for hiding this comment

maghamed Feb 27, 2019

Choose a reason for hiding this comment

maghamed Feb 26, 2019

Choose a reason for hiding this comment

Stepa4man Feb 27, 2019

Choose a reason for hiding this comment

maghamed Feb 26, 2019

Choose a reason for hiding this comment

Stepa4man Feb 27, 2019

Choose a reason for hiding this comment

maghamed left a comment

Choose a reason for hiding this comment

magento-cicd2 commented Feb 27, 2019 • edited Loading

ghost commented Mar 2, 2019

Stepa4man commented Feb 12, 2019 •

edited

Loading

osrecio Feb 12, 2019 •

edited

Loading

osrecio Feb 12, 2019 •

edited

Loading

maghamed Feb 26, 2019 •

edited

Loading

Stepa4man Feb 27, 2019 •

edited

Loading

magento-cicd2 commented Feb 27, 2019 •

edited

Loading