Injection Inception, Change #5: Add Access and Auth objects to DI #8026
Conversation
diosmosis
added
c: Platform
| Access::getInstance()->setSuperUserAccess(true); | ||
| }) | ||
| )), | ||
| )); |
There was a problem hiding this comment.
Just to be sure I understand this correctly: the container is already created at that point so in theory this wouldn't work. But in this command we include index.php so it will re-create a whole environment (and container), so this works. Is that right?
There was a problem hiding this comment.
The container is created in Console, then RequestCommand creates a new one for RequestCommand stuff (mainly because the Cache is created in UrlHelper), then it includes index.php which creates the environment again. And the last environment creation is where we want to set superuser access.
When the web app frontend is encapsulated, it will look a lot better, eg:
$webApp = new WebApplication();
$webApp->getEnvironment()->getContainer()->get('Piwik\Access')->setSuperUserAccess(true);
$webApp->dispatch(...);
|
Looking good, once #8008 is merged the diff should be simpler to read (going commit by commit was easy). The Also previously plugins had to do |
It was never documented so it shouldn't be part of the public API. It's used by a PRO plugin, though.
It shouldn't. LoginLdap actually only uses DI, so I think it will work both ways.
I'm not sure what the status of DI is as it relates to 3rd party plugin development, so I don't have an opinion on this. |
Good! Currently 3rd party plugins shouldn't use DI config, but being able to use it for our plugins is great. Nice change! |
|
I think this PR should be rebased on master now that #8008 has been merged -> the PR contains commits of #8008. I guess #8008 might have been rebased after this branch was created, so this branch contains the old commits (of before the rebase). Be careful when rebasing: commits of #8008 should be skipped so that they are removed from this branch (else they'll be applied twice). |
|
I rebased both branches when rebasing #8008, so should be ok. |
… password/password hash (which can be set to previous values in tests during local tracking) and allow these values to be set to null in corresponding set methods of Login's Auth class.
…removed when the environment is recreated. So now in RequestCommand when the container is recreated, the Access instance is destroyed and the doAsSuperUser call is negated.
|
After this is merged, this PR in the CustomAlerts repo must be merged as well: matomo-org/plugin-CustomAlerts#15 |
…ce the controllers check for it.
Injection Inception, Change #5: Add Access and Auth objects to DI
Fixing tests for matomo-org/matomo#8026 (DI related refactor)
This PR is based off of #8008, so that PR must be merged first.
This PR includes the following changes: