Omit the need to call all modifiers. #11
Conversation
src/Event/RequestSubscriber.php
Outdated
|
|
||
| // Eliminate the need to get and iterate through all the modifiers as it is | ||
| // a very resource intensive process. | ||
| $provider = Provider::load(self::GROUP_PURL_PROVIDER); |
There was a problem hiding this comment.
We can not just call the GROUP_PURL_PROVIDER as we will soon have multiple providers for domain in addition to path prefix, also purl should not be dependent on group_purl.
src/Event/RequestSubscriber.php
Outdated
| $provider_plugin = $this->provider->getProviderPlugin(); | ||
| } | ||
| if ($provider_plugin) { | ||
| $modifier = $provider_plugin->getModifierDataByKey($uri); |
There was a problem hiding this comment.
The uri will not always be the key for all providers/modifiers, the providers will need the entire request to make their determination.
I also doubted it that's why thought to get validation on this. anyways will see something else tomorrow in the same direction. |
| @@ -41,7 +41,7 @@ public function getProviderModifiersByKey(Provider $provider, $key) | |||
| { | |||
| $modifiers = []; | |||
|
|
|||
| if (method_exists($provider->getProviderPlugin(), 'getModifierDataByValue')) { | |||
| if (method_exists($provider->getProviderPlugin(), 'getModifierDataByKey')) { | |||
There was a problem hiding this comment.
still need this I think, because inside the if it calls getModifierDataByKey
https://github.com/openscholar/openscholar/issues/14405
I thought it would be better if we can totally omit the need to call all the modifiers rather them calling them all once and caching (that would still mean that every time cache is invalidated we need to query the DB multiple times), when we know that modifier is to be matched from the uri, why not use the URI to get the modifier and just match that rather than getting all modifiers by querying the DB (as many times as is the vsite count on the install) and then matching them one by one.
This does need some changes in
group purl moduletoo for which I am in process of creating patches and also am testing out certain aspects and fixing some notices being thrown. (This did need some thought and trying out multiple things to get everything working as before, but for this PR more or less this should be final code.)Raising this PR so that this can be reviewed till then and potential side effects can be assessed which can impact currently or in future.
@rbran100 @Bilsi @jashish24