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

Limited data loss at settings tables when migrating from a version smaller than 3.3.0 #10249

Closed
jonasraoni opened this issue Jul 25, 2024 · 4 comments · Fixed by #10256, pkp/ojs#4379, pkp/omp#1658, pkp/ops#739 or pkp/ops#740
Assignees
Labels
Bug:3:Critical A bug that prevents a substantial minority of users from using the software.
Milestone

Comments

@jonasraoni
Copy link
Contributor

jonasraoni commented Jul 25, 2024

Describe the bug
The resolution of the issue #9487 created a critical issue for users upgrading from an older application (< 3.3.0).

The line is attempting to grab the foreign key (e.g. submission_id, genre_id, etc), by assuming that the first column will always contain the field:
https://github.com/pkp/ojs/blob/8517ea2ddd6a88ed713067b81824a2495549df45/classes/migration/upgrade/OJSv3_3_0UpgradeMigration.inc.php#L165

But after the update below, the expectation was broken:
https://github.com/pkp/ojs/blame/8517ea2ddd6a88ed713067b81824a2495549df45/classes/migration/upgrade/OJSv3_3_0UpgradeMigration.inc.php#L114

The result is that the update clause will use the setting_name as key instead of the id, this will make all settings with the same setting_name to share the same value.

It's better to not trust in the order of columns and use a proper field name, as users might also change the order.

EDIT: After some analysis, I'd say the surface area is limited to the profileImage of users. External plugins are out of the analysis, but I'd guess few of them make use of such object field.

As the profile image ends with the user id, it's possible to recover the images with a migration, by searching for a file that follows the name pattern profileImage-{$userID}.ext.

What application are you using?
OJS 3.3

@jonasraoni jonasraoni added the Bug:3:Critical A bug that prevents a substantial minority of users from using the software. label Jul 25, 2024
@jonasraoni jonasraoni added this to the 3.3.0-19 milestone Jul 25, 2024
@jonasraoni jonasraoni changed the title https://github.com/pkp/pkp-lib/issues/9487 Possible data loss at settings tables Jul 25, 2024
@jonasraoni jonasraoni self-assigned this Jul 26, 2024
@jonasraoni
Copy link
Contributor Author

jonasraoni commented Jul 26, 2024

Looks like the surface of this issue isn't big. The object setting isn't used often, some entities which are managed by the schema/JSON (site_settings, announcement_settings, author_settings, publication_galley_settings, journal_settings, email_templates_settings, publication_settings, submission_settings) and the entities plugin_settings and review_form_element_settings are not going to be affected.

The setting entities which remain, and barely make use of object settings, are the following:

Common for all applications

  • announcement_type_settings
  • category_settings
  • citation_settings
  • controlled_vocab_entry_settings
  • data_object_tombstone_settings
  • event_log_settings
  • filter_settings
  • genre_settings
  • library_file_settings
  • metadata_description_settings
  • navigation_menu_item_assignment_settings
  • navigation_menu_item_settings
  • notification_settings: I found a case, but looks like it's a bug (setting name contents, should be a string, but it's an object with the following content {"oldPassword":"The current password you entered is invalid"}), perhaps an old one from an OJS 2 installation
  • notification_subscription_settings
  • review_form_settings
  • submission_file_settings
  • user_group_settings
  • user_settings: The profileImage makes use of an object setting

OJS specific

  • issue_galley_settings
  • issue_settings
  • section_settings
  • static_page_settings
  • subscription_type_settings

OMP specific

  • publication_format_settings
  • series_settings
  • spotlight_settings
  • static_page_settings
  • submission_chapter_settings

OPS specific

  • section_settings

@jonasraoni
Copy link
Contributor Author

jonasraoni commented Jul 26, 2024

The following MySQL query will generate another query that might be suitable to detect the cases where a data loss happened.

Execute it, copy all the rows of the response (they will be SQL commands), remove the trailing UNION ALL at the end and run the query again:

SELECT CONCAT('SELECT ', QUOTE(TABLE_NAME), ', setting_name, setting_value, COUNT(0) FROM ', TABLE_NAME, ' WHERE ', COLUMN_NAME, ' = ', QUOTE('object'), ' GROUP BY setting_name, setting_value HAVING COUNT(0) > 1 UNION ALL ')
FROM information_schema.COLUMNS x
WHERE COLUMN_NAME = 'setting_type'
AND TABLE_NAME NOT IN (
    'review_form_element_settings', 'plugin_settings',
    'site_settings', 'announcement_settings', 'author_settings', 'publication_galley_settings', 'journal_settings', 'press_settings', 'server_settings', 'email_templates_settings', 'publication_settings', 'submission_settings'
)
AND EXISTS (SELECT 0 FROM information_schema.COLUMNS y WHERE y.TABLE_NAME = x.TABLE_NAME AND y.TABLE_SCHEMA = x.TABLE_SCHEMA AND y.COLUMN_NAME = 'setting_name')

If you have many OJS/OPS/OMP databases on the same server, add AND TABLE_SCHEMA = 'your_database_name' to the end of the query.

@jonasraoni jonasraoni changed the title Possible data loss at settings tables Data loss at settings tables when migrating from a version smaller than 3.3.0 Jul 26, 2024
@jonasraoni jonasraoni changed the title Data loss at settings tables when migrating from a version smaller than 3.3.0 Limited (profile image of users) data loss at settings tables when migrating from a version smaller than 3.3.0 Jul 26, 2024
@jonasraoni jonasraoni changed the title Limited (profile image of users) data loss at settings tables when migrating from a version smaller than 3.3.0 Limited data loss at settings tables when migrating from a version smaller than 3.3.0 Jul 28, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 28, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 28, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 28, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 28, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 28, 2024
@jonasraoni
Copy link
Contributor Author

@asmecher
Copy link
Member

@jonasraoni, thanks, I added a couple comments on the OJS PR.

jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ops that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ops that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Jul 31, 2024
(cherry picked from commit 53c1fbc)
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ops that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ops that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit that referenced this issue Jul 31, 2024
…ix-data-loss-on-settings

#10249 Added migration to recover from data loss on the pr…
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ops that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Jul 31, 2024
jonasraoni added a commit to pkp/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ops that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ops that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/omp that referenced this issue Jul 31, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Aug 5, 2024
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Aug 5, 2024
jonasraoni added a commit to jonasraoni/ojs that referenced this issue Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment