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
Restore auth when calling API only if needed #7804
Conversation
Piwik::postEvent('API.Request.authenticate', array($tokenAuth)); | ||
Access::getInstance()->reloadAccess(); | ||
SettingsServer::raiseMemoryLimitIfNecessary(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's maybe a matter of taste but I'm not a huge fan of reloadAuthUsingTokenAuth()
actually calling doReloadAuthUsingTokenAuth()
, it's a bit redundant and makes the code harder to navigate (eg. to illustrate what prevents from having another doReallyReloadAuthUsingTokenAuth()
).
Maybe something like that?
public static function reloadAuthUsingTokenAuth()
{
if (! self::shouldReloadAuthUsingTokenAuth($request)) {
return;
}
// do the thing
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can rename it to doReallyReloadAuthUsingTokenAuth
. That method is called by reloadAuthUsingTokenAuth
but also when restoring the token. Here we definitely want to reload auth even shouldReloadAuthUsingTokenAuth
would return false
. That's why I added that method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed it to forceReloadAuthUsingTokenAuth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 better choice
Except the minor comment 👍 Did you see a significant performance difference? |
Restore auth when calling API only if needed
Also added tests, there were none before...