Allow custom cleaners #23
Conversation
| $this->migrateTableTriggers($setName, $tableCollection); | ||
| } | ||
|
|
||
| public function registerCustomCleaner(FieldCleaner $cleaner, string $alias): void |
There was a problem hiding this comment.
ideally we'd be able to remove this in a future PR. i have a suspicion that some extra work on refactoring the Migrator and App classes (and adding a well-designed Config object) will show us a better path
| $destConnection->exec('DROP TABLE IF EXISTS ' . $sourceConnection->quoteIdentifier($table)); | ||
|
|
||
| $driverName = $sourceConnection->getDriver()->getName(); | ||
| if ($driverName === 'pdo_mysql') { |
There was a problem hiding this comment.
in a future PR i think i'd like to add a DatabaseDriver interface, with separate concrete classes for MySQL and SQLite. This would make supporting other db drivers (such as Postgres) far easier
| $this->destination->insert($tableName, $row); | ||
| } | ||
|
|
||
| public function postWrite(): void |
There was a problem hiding this comment.
i don't like this method being here, but until the Migrator is refactored a bit more I think it's a necessary evil
|
i think the unit test failure in https://travis-ci.org/github/MapleSyrupGroup/dbsampler/builds/752264598 will go away once we're building on current versions of PHP (certainly doesn't throw on my local |
|
Once #20 is closed I'll update this PR with the up-to-date master |
Our use-case is that we need to register custom cleaners into the migration. I've refactored (as much as is necessary to achieve my goal, although there is a lot more that could / should be done) to decouple sampling from cleaning and writing, and now use the migrator as a controller / manager to orchestrate the process. Decoupling has allowed me to remove the
CleanAllandCleanMatchedclasses - you can now just useAllandMatchedand pass through acleanfieldsdirective in your stanza.I've removed a lot of the public methods from classes' APIs, and preferred injection over setters. I've loosely encapsulated some of the config into objects, but there is more that can be done there. I think that if all the config is encapsulated and typed throughout, then there will be some pretty obvious low-hanging fruit to make some immediate architectural improvements.
I think that with a bit more work either the
ApporMigratorclass will become redundant. That's beyond the scope of this PR - i might do that in a future PR. I've added a bunch of tests. TheMigratorandAppclasses are pretty hard to test right now, so this suggests that there is still a lot of work to be done in there...This PR depends heavily on the changes I've introduced in #19, #20, and #21 (as i've branched from my local master which already incorporates that work).