2

enter image description here

I am not getting the implementation of Cleanup Probability settings which you can find at Stores > Configuration > Services > OAuth > Cleanup Settings > Cleanup Probability.

From the Magento docs, it has below definition

Specifies the number of OAuth requests before cleanup is launched. Do not enter 0 to disable cleanup.

But if you check it's usage, you will see below code

Method: \Magento\Integration\Model\Oauth\Token::afterSave()

public function afterSave()
{
    parent::afterSave();

    // Cleanup old entries
    if ($this->_oauthData->$this->_oauthData->isCleanupProbability()()) {
        $this->_getResource()->deleteOldEntries($this->_oauthData->getCleanupExpirationPeriod());
    }
    return $this;
}

So whenever a token saves happens, then if the condition $this->_oauthData->isCleanupProbability() met, then it will remove then old token entries.

Now checkout the isCleanupProbability() method.

Method: \Magento\Integration\Helper\Oauth\Data::isCleanupProbability()

public function isCleanupProbability()
{
    // Safe get cleanup probability value from system configuration
    $configValue = (int)$this->_scopeConfig->getValue(self::XML_PATH_CLEANUP_PROBABILITY);
    return $configValue > 0 ? 1 == \Magento\Framework\Math\Random::getRandomNumber(1, $configValue) : false;
}

The default value of $configValue above is 100. That means the above method depends on 1 == \Magento\Framework\Math\Random::getRandomNumber(1, 100) expression. The chance of becoming this condition true is rare.

So basically, removing expired tokens based on a random condition does not make sense to me. I can update this setting to 1 which will always trigger the removal process. I feel this settings needs to be a select box with values "Yes", "No" and do the operation accordingly.

Or we dont need this checking here at all. This operation makes sense in a cron job and do not need to do in a token save operation.

If you see any other valid usage of this setting, then please share it here. Cheers!

2
  • 1
    I agree with your assessment. In addition, as you noted, this field's comment seems incongruous with the function itself. This makes me think development of this feature started down one path and ended on another. Perhaps it's even been abandoned in some respect, since it will almost never fire. It might be worth filing a report with Magento's Github project for clarification, improvement, or retirement of this feature. As near as I can tell, right now, it's just a minuscule but recurring waste of processing time. – kookaburra May 24 at 14:06
  • 1
    @kookaburra My thought was exactly the same and this question is asked in order to make sure my assessment is right. Yes, this needs to be reported in the Github if in case I am not getting any valid answer. Thanks for sharing your thoughts. Really appreciate it. – Rajeev K Tomy May 24 at 16:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.