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

Introduce replacement schema management toolset (replacing ADODB XMLSchema) #2493

Closed
4 tasks done
asmecher opened this issue May 4, 2017 · 16 comments
Closed
4 tasks done
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.

Comments

@asmecher
Copy link
Member

asmecher commented May 4, 2017

Introduce replacement toolset for ADODB.

  • Core schema management for installation
  • Introduce migrations for core upgrades
  • Schema creation for plugins
  • Upgrades for plugins [not currently supported but will be necessary for future plugin schema changes] [Edit: This was already present but never used! Grep PLUGIN_UPGRADE_FILE.] [Also apparently never worked.]
@asmecher
Copy link
Member Author

asmecher commented May 4, 2017

On tools: #2163 has introduced Laravel's query builder. That framework also has a Migrations toolset (https://laravel.com/docs/5.4/migrations).

Looking at comparables, there's also the standalone Phinx migration toolset; there's some discussion of the complications of using Laravel migrations outside of the Laravel framework, and using Phinx instead, that's directly relevant to our needs: https://www.reddit.com/r/PHP/comments/4g9g3n/using_illuminatedatabase_outside_of_laravel/

@asmecher
Copy link
Member Author

asmecher commented May 4, 2017

On migrations: The most difficult aspect of this is probably going to be flipping the upgrade process to something that doesn't require ADODB.

Suggestion: we start coding using migrations tools, invoked from the upgrade script using <code> elements (or a new useful shorthand element like that) and avoiding dependence on upgrade steps that depend on ADODB tools like schema XML syncs. Once that's been maintained for a few releases, we can cut off upgrade compatibility at the same time we ax ADODB. We can synchronize this with a major release where upgrade cutoffs might be expected anyway.

@asmecher
Copy link
Member Author

asmecher commented May 4, 2017

@bozana, I'd love your input on this! Just brainstorming at the moment.

@asmecher
Copy link
Member Author

asmecher commented May 4, 2017

Another contender, which (at a glance) has ADODB-like table definitions in XML: http://propelorm.org/ (plus some relevant Propel/Phinx discussion from a while ago)

@bozana
Copy link
Collaborator

bozana commented May 8, 2017

I will do some research and thinking... :-)

@netsensei
Copy link

netsensei commented Nov 8, 2019

Hi,

We are in the process of migrating from MySQL to PostgreSQL. I was just testing if DB upgrades are working on PostgreSQL when I ran into issue #1793. This issue suggests patching the ADODB library which isn't ideal.

So, that got me thinking.

I was wondering: have you looked into Doctrine/DBAL?

https://www.doctrine-project.org/projects/dbal.html
https://www.doctrine-project.org/projects/doctrine-dbal/en/2.9/reference/introduction.html#introduction

Both Propel (http://propelorm.org/) and Laravel's Eloquent (https://laravel.com/docs/5.8/eloquent) (which offers a migration toolkit) are fully fledged ORM libraries. The Doctrine Project offers it's own ORM alternative aptly named "ORM" (https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/index.html)

Looking at OJS, my assessment would be that moving towards a fully fledged ORM would certainly bring many benefits and simplifications, but it's definitely a big challenge as it would require major architectural changes.

DBAL might be an intermediate solution. It would allow OJS to isolate database related logic and provide a nicely abstracted API that can be used, without necessarily being shoe-horned into OOP models that represent and track database state.

Further making the case for DBAL:

Schema manager:
https://www.doctrine-project.org/projects/doctrine-dbal/en/2.9/reference/schema-manager.html

  • Represent your schema in XML or YAML and use that to create databases
  • createSchema() allows you to compare schema's and calculate SQL queries for an upgrade path.

Platforms:
https://www.doctrine-project.org/projects/doctrine-dbal/en/2.9/reference/platforms.html#platforms

  • Out of the box support for many different platforms including MS SQL, MySQL, MariaDB, PostgreSQL, SQLite, Oracle SQL

Transactions
https://www.doctrine-project.org/projects/doctrine-dbal/en/2.9/reference/transactions.html#transactions

  • PDO like transactions with rollback options and auto-commit mode

Query builder
https://www.doctrine-project.org/projects/doctrine-dbal/en/2.9/reference/query-builder.html#sql-query-builder

Here's an example application I wrote that leverages DBAL for managing a MySQL to MS SQL migration: https://github.com/VlaamseKunstcollectie/tmssync

So, a potential approach might look like this:

Taking a stab at the OJS code, I'd reckon that classes\db\DAO.inc.php would be your point of entry where you'd hook DBAL up into such functions like update and retrieve. You're also going to have to look at classes\db\DBConnection.inc.php and classes\db\SQLParser.inc.php.

Basically, you would keep the function signatures in those classes intact, but you'd replace references to ADODB with DBAL. That way, you'd avoid touching the rest of the codebase at this point. Testing whether it works would be by yanking out the entire ADODB library and all related "require" or "import" statements and see how much breaks.

Finally, you want to gradually strip out OJS code that's doing what DBAL already offers out of the box such as creating prepared statements, sanitising SQL queries, etc.

Regardless, I'm aware that replacing ADODB is a fundamental change to implement. So, it's not something I expect to land anytime soon.

@asmecher
Copy link
Member Author

asmecher commented Nov 8, 2019

@netsensei, thanks, that's a very thoughtful summary of a bunch of good options.

I'm not sure if you've encountered OJS's service classes yet (e.g. https://github.com/pkp/pkp-lib/blob/master/classes/services/PKPSubmissionService.inc.php), and their associated query builder classes (https://github.com/pkp/pkp-lib/blob/master/classes/services/queryBuilders/PKPSubmissionQueryBuilder.inc.php) -- these are new tools that are gradually displacing what used to be coded in the DAOs. Our goal is to gradually move queries over to the querybuilder pattern, then shift the additional elements like schema management and upgrade processes over to a migrations-based toolset. And then we can get rid of ADODB. This will be quite a long process, but aspects of it are already well underway!

@netsensei
Copy link

Okay! That's great news!

I haven't seen those service classes yet. Looks like you're currently leveraging illuminate\database. And I can see that these are only first steps as, for instance, PKPAuthorService doesn't get yet instantiated elsewhere, right?

Consolidating your database interfacing logic into service classes is definitely a big step forward. I also notice there's work being done moving towards a PSR-4 compliant DI service container support to manage dependencies which is another good thing.

Keeping tabs on this!

ajnyga added a commit to ajnyga/ops that referenced this issue Apr 7, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue May 8, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue May 9, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue May 9, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue May 9, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue May 9, 2020
asmecher added a commit to asmecher/ojs that referenced this issue May 9, 2020
asmecher added a commit to asmecher/ojs that referenced this issue May 9, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue May 11, 2020
asmecher added a commit that referenced this issue Jun 8, 2020
asmecher added a commit to pkp/ojs that referenced this issue Jun 8, 2020
asmecher added a commit to pkp/ojs that referenced this issue Jun 8, 2020
asmecher added a commit to pkp/ojs that referenced this issue Jun 8, 2020
asmecher added a commit to pkp/omp that referenced this issue Jun 8, 2020
asmecher added a commit to pkp/ojs that referenced this issue Jun 8, 2020
asmecher added a commit to pkp/ops that referenced this issue Jun 8, 2020
asmecher added a commit to pkp/omp that referenced this issue Jun 8, 2020
asmecher added a commit to pkp/ojs that referenced this issue Jun 8, 2020
ppv1979 added a commit to ppv1979/pkp-lib that referenced this issue Jun 10, 2020
asmecher pushed a commit that referenced this issue Jun 10, 2020
asmecher added a commit to pkp/ojs that referenced this issue Aug 27, 2020
asmecher added a commit that referenced this issue Aug 27, 2020
…econciled 3.0.x/3.2.x upgrade expectations
asmecher added a commit that referenced this issue Aug 27, 2020
asmecher added a commit to pkp/ojs that referenced this issue Aug 27, 2020
asmecher added a commit to pkp/omp that referenced this issue Aug 27, 2020
asmecher added a commit to pkp/ojs that referenced this issue Aug 27, 2020
asmecher added a commit to pkp/omp that referenced this issue Aug 27, 2020
@asmecher asmecher mentioned this issue Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.
Projects
None yet
Development

No branches or pull requests

4 participants