Improve dependency injection
Class names should not be part of options of any component - the application (for now, just the Symfony commands) is entirely responsible for constructing concrete objects and passing them to the Task
component. Where components need to construct objects dynamically, a factory class should be passed.
The todos here:
- Move all of
EtlTask
's construction logic intoEtlCommand
. - Introduce
DataRecord
andDataProperty
factories, and inject factory instances wherever we currently construct records and properties. - Look at
KeyMapLookup
's means of obtaining theKeyMap
instance(s) to reference - how best to inject them? Note we'll be implementing a filter using the key map as well. So - perhaps pass in a key map factory? - The 'transform' portion of the task configuration should probably be a class containing a list of mappings, keyed by destination property name and containing another class which is a transformer list.
Original issue text:
Review our use of dependency injection (and lack thereof). We have a number of places that are using specific component implementations. Note that all known cases below are using them to call static functions, so object instances don't need to be injected, just class names.
- All transformers that actually transform the input data use
Property::create()
to create the return value, they should inject the class name. -
Soong\Data\Record::create()
hard-codes its own class name, should usestatic
. More seriously, it hard-codesProperty
to create properties to populate the record - the class name should actually be injected here. Similarly fornullProperty()
. Although... perhaps it would be better if instead of creating property instances up front and storing them in the record, we just stored the raw data and left it togetProperty()
to wrap them in property objects on retrieval? Would want to cache the objects though... -
Soong\Extractor\ArrayExtractor
hard-codesRecord::create()
, should inject the class name. -
Soong\Csv\Loader
uses hard-codedProperty
class to make a serial int destination ID property. Actually, why do that at all? -
Soong\DBAL\Loader
uses hard-codedProperty
class to store the last inserted ID in the destination key. Should inject the class. -
Soong\Transformer\KeyMapLookup
uses a hard-codeEtlTask
to get the task identified bytask_id
. This is a case where we should figure out how to inject the referenced task object itself. - Console command implementations (
MigrateCommand
et. al.) use theSoong\Task\Task
implementation directly. Note that they only call the staticgetTask()
method, so do not need a task object injected - this is a place where injecting a class name suffices. And, this is high enough in the hierarchy (called directly by the outer application) that it's not horrible to be opinionated on implementation here. Not a priority to change this. - We inject record classes as 'record_class' in some places, and 'data_record_class' elsewhere. We need to consolidate this.
Edited by Mike Ryan