berlindb / core

All of the required core code
MIT License
252 stars 27 forks source link

Version of DB tables shall be deleted if the tables doesnot exist with uninstall action #108

Closed Mai-Saad closed 3 years ago

Mai-Saad commented 3 years ago

Precondition:

Steps: 1- Manually Delete those tables from the database without deleting related options 2- Call the uninstall method here https://github.com/berlindb/core/blob/20d2542036fa6423c55c38af0581e8340e69f7c7/table.php#L306-L313

Expected: Version options related to those tables are deleted

Actual: Version options aren't deleted

Impact: With the next install, with the existing version and no tables=> those tables will never be created again unless if options were deleted manually from the database

Proposed solution:

JJJ commented 3 years ago

Hey @Mai-Saad! Thank you for sharing your experience and ideas here. 👋

This is an interesting issue, because the primary function of those options is to avoid checking if a table exists on every page load. When the tables are deleted without the options, how could BerlinDB know?

There basically is no built-in protection against a Table/Option mismatch.

Also see #105 for some related thoughts.

Similarly to uninstall(), the create() call inside install() could fail if it is called manually and the table already exists. Should set_db_option() only be called on confirmed successful creation? I think yes, because there is no way to confirm that the tables that exist are BerlinDB's, and up-to-date if so.

Will work up a PR for this.

JJJ commented 3 years ago

PR merged. Issue fixed and closed! Thanks again @Mai-Saad 🙏

Mai-Saad commented 3 years ago

Thank you @JJJ