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 into
DataPropertyfactories, and inject factory instances wherever we currently construct records and properties.
- Look at
KeyMapLookup's means of obtaining the
KeyMapinstance(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 use
static. More seriously, it hard-codes
Propertyto create properties to populate the record - the class name should actually be injected here. Similarly for
nullProperty(). 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 to
getProperty()to wrap them in property objects on retrieval? Would want to cache the objects though...
Record::create(), should inject the class name.
Propertyclass to make a serial int destination ID property. Actually, why do that at all?
Propertyclass to store the last inserted ID in the destination key. Should inject the class.
Soong\Transformer\KeyMapLookupuses a hard-code
EtlTaskto get the task identified by
task_id. This is a case where we should figure out how to inject the referenced task object itself.
- Console command implementations (
MigrateCommandet. al.) use the
Soong\Task\Taskimplementation directly. Note that they only call the static
getTask()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.