Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a migration step to save the data from the accounts table before … #4813

Merged
merged 7 commits into from
May 18, 2017

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented May 11, 2017

…migrating

  • Copy data
  • Make sure you can upgrade from 10.0.0
  • Copy avatars to new location
  • Recreate accounts table with our data

Signed-off-by: Joas Schilling coding@schilljs.com

@mention-bot
Copy link

@nickvergessen, thanks for your PR! By analyzing the history of the files in this pull request, we identified @LukasReschke, @rullzer and @MorrisJobke to be potential reviewers.

@nickvergessen nickvergessen force-pushed the accounts-table-migration-from-owncloud branch from d866c08 to 624f894 Compare May 12, 2017 07:40
@codecov
Copy link

codecov bot commented May 12, 2017

Codecov Report

Merging #4813 into master will decrease coverage by 0.06%.
The diff coverage is 2.67%.

@@             Coverage Diff              @@
##             master    #4813      +/-   ##
============================================
- Coverage     54.12%   54.05%   -0.07%     
- Complexity    22206    22234      +28     
============================================
  Files          1367     1368       +1     
  Lines         84983    85061      +78     
  Branches       1322     1322              
============================================
- Hits          45998    45981      -17     
- Misses        38985    39080      +95
Impacted Files Coverage Δ Complexity Δ
.../private/Repair/Owncloud/SaveAccountsTableData.php 0% <0%> (ø) 20 <20> (?)
...b/private/Repair/NC11/MoveAvatarsBackgroundJob.php 0% <0%> (ø) 21 <11> (+9) ⬆️
version.php 0% <0%> (ø) 0 <0> (ø) ⬇️
lib/private/Repair.php 28.2% <0%> (-0.75%) 19 <0> (ø)
lib/private/Updater.php 6.2% <23.07%> (+0.42%) 77 <0> (+4) ⬆️
apps/comments/lib/EventHandler.php 79.16% <0%> (-8.34%) 7% <0%> (ø)
apps/theming/lib/Capabilities.php 71.42% <0%> (-6.35%) 2% <0%> (-1%)
lib/private/Files/Cache/Propagator.php 94.93% <0%> (-1.27%) 16% <0%> (ø)
lib/private/Security/CertificateManager.php 90.81% <0%> (-1.03%) 39% <0%> (ø)
core/js/js.js 61.21% <0%> (-0.57%) 0% <0%> (ø)
... and 7 more

@MorrisJobke
Copy link
Member

I get this when running an update from OC 10 to NC master:

Failed to update database structure (Doctrine\\DBAL\\Schema\\SchemaException: There is no column with name 'uid' on table 'oc_accounts'. in 3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Schema/SchemaException.php:86\nStack trace:
    #0 3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Schema/Table.php(671): Doctrine\\DBAL\\Schema\\SchemaException::columnDoesNotExist('uid', 'oc_accounts')
    #1 3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php(702): Doctrine\\DBAL\\Schema\\Table->getColumn('uid')
    #2 3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php(630): Doctrine\\DBAL\\Platforms\\MySqlPlatform->getPreAlterTableAlterPrimaryKeySQL(Object(Doctrine\\DBAL\\Schema\\TableDiff), Object(Doctrine\\DBAL\\Schema\\Index))
    #3 3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php(612): Doctrine\\DBAL\\Platforms\\MySqlPlatform->getPreAlterTableIndexForeignKeySQL(Object(Doctrine\\DBAL\\Schema\\TableDiff))
    #4 3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Schema/SchemaDiff.php(199): Doctrine\\DBAL\\Platforms\\MySqlPlatform->getAlterTableSQL(Object(Doctrine\\DBAL\\Schema\\TableDiff))
    #5 3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Schema/SchemaDiff.php(136): Doctrine\\DBAL\\Schema\\SchemaDiff->_toSql(Object(Doctrine\\DBAL\\Platforms\\MySQL57Platform), false)
    #6 lib/private/DB/Migrator.php(254): Doctrine\\DBAL\\Schema\\SchemaDiff->toSql(Object(Doctrine\\DBAL\\Platforms\\MySQL57Platform))
    #7 lib/private/DB/Migrator.php(86): OC\\DB\\Migrator->applySchema(Object(Doctrine\\DBAL\\Schema\\Schema))
    #8 lib/private/DB/MDB2SchemaManager.php(122): OC\\DB\\Migrator->migrate(Object(Doctrine\\DBAL\\Schema\\Schema))
    #9 lib/private/legacy/db.php(186): OC\\DB\\MDB2SchemaManager->updateDbFromStructure('/srv/projects/s...')
    #10 lib/private/Updater.php(288): OC_DB::updateDbFromStructure('/srv/projects/s...')
    #11 lib/private/Updater.php(237): OC\\Updater->doCoreUpgrade()
    #12 lib/private/Updater.php(130): OC\\Updater->doUpgrade('12.0.0.20', '10.0.0.12')
    #13 core/ajax/update.php(197): OC\\Updater->upgrade()
    #14 {main})

Any idea why it fails with this error, because it properly detects that this needs to be added.

@nickvergessen
Copy link
Member Author

Grml, Maria DB? I think we need to kill the table completly because it tries to update the primary index instead of deleting + recreating. But that doesn't work, because it still contains wrong columns after adding and before removing of the old columns

@MorrisJobke
Copy link
Member

Grml, Maria DB?

No - MySQL 5.7.18 on Ubuntu 16.04

@nickvergessen
Copy link
Member Author

Fixed, also adjusted the background job that copies the avatars to work with ownclouds new location.

@nickvergessen
Copy link
Member Author

@schiessle please help with generating the account table entries

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. 👍 Avatars got moved for me

…migrating

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Member Author

So aparently the account entries are created on next usage then. So should be fine.

@nickvergessen nickvergessen force-pushed the accounts-table-migration-from-owncloud branch from 4833862 to 682a57d Compare May 18, 2017 09:28
@nickvergessen
Copy link
Member Author

Rebased for version.php change

@LukasReschke LukasReschke merged commit 10930c9 into master May 18, 2017
@LukasReschke LukasReschke deleted the accounts-table-migration-from-owncloud branch May 18, 2017 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants