-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
Dependency inversion and AR #16
Comments
Relations may use different repository/connection. In addition such dependency will make caching of AR really tricky... |
They won't use a different repository; think of the repository as a factory or service locator for just DB connections.
Why would it become trickier? The only thing that changes is the way we retrieve the connection, you can still use caching in the exact same way? --> I realize with AR you can never have it be totally pure, but this approach would allow us to use this library without having global contants like |
It may depend on
Am I missing something obvious? |
(I'm making this up as I go) [
'db1' => [normal db config],
'db2' => [normal db config],
'db3' => [normal db config]
'mainRepo' => [
'connections' => [
Reference::to('db1'),
Reference::to('db2'),
Reference::to('db3'),
]
] What's relevant here is the fact that the consuming code gets the repository injected and then manually injects it into AR. public function actionX(Repository $repository)
{
$car = new Car($repo);
} Should we be caching the AR models themselves? Or just the underlying queries? If serialization of AR models is a requirement then you are right we have an issue, as anything with an injected dependency cannot simply be serialized / unserialized. In case of AR, we could have the |
To make this clean we would need to remove some magic from AR and remove direct dependency on connection:
But it makes using AR less convenient and more error-prone - IMO it will create more problems than it solves. |
Historically in Yii2 one of the parts where it is really hard to inverse dependencies is AR.
The current AR implementation uses a static factory
Car::find()
which creates a query object.The model depends on a database connection, which is found via:
The follows directly from the AR pattern. An AR object needs to know how to persists itself (it essentially violates SRP by design).
Currently there's 2 main ways of using AR, creation and searching:
These things are primarily called inside controller actions, where we (should?) have DI available. This means that it could be feasible to remove the
Yii::getApp()
dependency as follows:Now of course this means that we always need a
Repository
when working with AR, but since we usually have this at the start, it should not be hard to get these and then pass it on.We make it available from the AR object so it can be reused when using relations or saving the object later.
Note this is a very raw idea, feedback, as always is welcomed.
The text was updated successfully, but these errors were encountered: