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

Remove AMP related data while uninstalling plugin. #6422

Merged
merged 20 commits into from
Jul 23, 2021

Conversation

dhaval-parekh
Copy link
Collaborator

Summary

Fixes #3210

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@github-actions
Copy link
Contributor

github-actions bot commented Jun 24, 2021

Plugin builds for 9951f6e are ready 🛎️!

@dhaval-parekh dhaval-parekh force-pushed the enhancement/3210-remove-data-on-uninstall branch from 475e4cb to c487ed2 Compare June 28, 2021 06:40
@dhaval-parekh dhaval-parekh requested a review from swissspidy June 28, 2021 06:48
@westonruter
Copy link
Member

It looks like there is more data we can purge:

  • The amp_css_transient_monitor_time_series option.
  • The amp_customize_setting_modified_timestamps theme mod.

There may be some others but I think that's all the persistent data.

Finally, after deleting all persistent data, we can also purge a lot of transient data, specifically when not using an external object caching plugin (wp_using_ext_object_cache() returns false). We can do a straight SQL DELETE from the options table for all transients related to the plugin:

  • All parsed CSS with the transient key beginning with amp-parsed-stylesheet-v.
  • All cached image dimensions beginning with amp_img_.
  • Validation errors URL count transient: amp_new_validation_error_urls_count.
  • Validation error index count transient: amp_error_index_counts.
  • (Optional) Validation errors generated during plugin activation with transient key: amp_plugin_activation_validation_errors. This has a 60 second expiration, so not important.
  • (Optional) The themes from WordPress.org stored in the amp_themes_wporg transient. This has a 1-day expiration, so not important.
  • (Optional) Image caching lock transients beginning with amp_lock_. This has a 60-second expiration, so not important.

This is particularly useful for users who have a bad experience with the AMP plugin when they have problems with the AMP plugin filling up their database with transients. By removing all of the transients upon uninstallation, their problem will be solved.

@dhaval-parekh dhaval-parekh force-pushed the enhancement/3210-remove-data-on-uninstall branch from 3ae73c3 to 3f7a4b5 Compare July 5, 2021 11:05
@dhaval-parekh dhaval-parekh self-assigned this Jul 21, 2021
@dhaval-parekh dhaval-parekh force-pushed the enhancement/3210-remove-data-on-uninstall branch from 3f7a4b5 to 80015e0 Compare July 21, 2021 09:57
@westonruter westonruter force-pushed the enhancement/3210-remove-data-on-uninstall branch from 80015e0 to ef7fa2b Compare July 21, 2021 20:52
@westonruter westonruter added this to the v2.2 milestone Jul 21, 2021
@dhaval-parekh dhaval-parekh force-pushed the enhancement/3210-remove-data-on-uninstall branch from 9694d52 to d2378e3 Compare July 22, 2021 14:09
@westonruter

This comment has been minimized.

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

I tested this by grabbing a SQL dump via wp db export --extended-insert=false before.sql and then I uninstalled and did another SQL dump via wp db export --extended-insert=false after.sql. Then I did diff before.sql after.sql and looked at the changes, all of which appear to be limited to the scope of what was intended to remove AMP data from the database.

@westonruter westonruter merged commit d46378b into develop Jul 23, 2021
@westonruter westonruter deleted the enhancement/3210-remove-data-on-uninstall branch July 23, 2021 23:28
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow removing all data on uninstall
3 participants