Skip to content
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

Moved isCurrentlySecure() method to Mage_Core_Model_App #3225

Merged
merged 4 commits into from
May 9, 2023

Conversation

colinmollenhour
Copy link
Member

I brushed the dust off of Cm_Diehard and it seems it was broken by one of my own PRs (#1462).. When generating the cache key before the full init, it checks the request scheme which tries to load the store which hasn't been initialized yet so throws Mage_Core_Model_Store_Exception.

This PR moves the isCurrentlySecure method (and keeps the old one as a wrapper for BC) to Mage_Core_Model_App so that it can be called earlier in the request cycle without causing a Mage_Core_Model_Store_Exception. I don't know if any other modules require this but there is no regression.

I also added a comment to "Offloader header" to clarify it's utility.

Just for kicks, here are the results with the "Local" backend on a clean installation with Cm_DiehardSample (Cm_Diehard on the left, no FPC on the right):

image

…cked without full config load (before store is initialized).
@github-actions github-actions bot added the Component: Core Relates to Mage_Core label Apr 28, 2023
fballiano
fballiano previously approved these changes May 9, 2023
ADDISON74
ADDISON74 previously approved these changes May 9, 2023
@colinmollenhour colinmollenhour dismissed stale reviews from ADDISON74 and fballiano via b6e6857 May 9, 2023 16:53
fballiano
fballiano previously approved these changes May 9, 2023
colinmollenhour and others added 2 commits May 9, 2023 12:55
Co-authored-by: Fabrizio Balliano <fabrizio.balliano@gmail.com>
@fballiano fballiano merged commit d42170e into OpenMage:main May 9, 2023
@fballiano
Copy link
Contributor

I merged to speed things up just because it already had 2 green checks and there were 2 minor docblock changes after that, but still all checks are ok.

@fballiano fballiano changed the title Allow app request to be initialized and isCurrentlySecure to be che… Moved isCurrentlySecure() method to Mage_Core_Model_App May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Relates to Mage_Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants