Are ready.php hooks/addHookMethods called outside their use?

[a-ok]

Are `addHookMethod` hooks, declared in ready.php fired when ready.php is read or when the hook is used?

The reason I ask is that I am combining a `addHookMethod` with an API (with a rate limit) but I'm only calling the hook in the head.inc and the page views are low but I've already hit the rate limit per month. If it's in ready.php is it being called on CMS page views etc too even if it's a hookMethod?

$ipapiHttp = new WireHttp();
$ipapiAPIKey = "XXX";

wire()->addHookMethod('Page::ipapiGetUserLocation', function($event) use($ipapiHttp, $ipapiAPIKey) {
	$page = $event->object;

	$client  = @$_SERVER['HTTP_CLIENT_IP'];
	$forward = @$_SERVER['HTTP_X_FORWARDED_FOR'];
	$remote  = $_SERVER['REMOTE_ADDR'];
	if (!empty($client)) {
		$ip = $client;
	} elseif (!empty($forward)) {
		$ip = $forward;
	} else {
		$ip = $remote;
	}

	$result = $ipapiHttp->getJSON("https://api.ipapi.com/$ip?access_key=$ipapiAPIKey");
	$event->return = $result;
});

And in head.inc I'm calling the hook method:

if (!$session->get("shopifyShopCurrency")) {
	if ($page->ipapiGetUserLocation()['currency']['code'] && $page->ipapiGetUserLocation()['currency']['code'] !== 'GBP') {
		$session->set("shopifyShopCurrency", "ROW");
	} elseif (!$session->get("shopifyShopCurrency")) { // Set it as the Shopify store default
		$session->set("shopifyShopCurrency", $shopifyShopCurrency);
	}
} else {
	$session->set("shopifyShopCurrency", $shopifyShopCurrency);
}

 

[wbmnfktr]

Quote

/site/ready.php
This file is included immediately after the API is fully ready. It behaves the same as a ready() method in an autoload module. The current $page has been determined, but not yet rendered. This is an excellent place to attach hooks that may need to know something about the current page. It's also an excellent place to include additional classes or libraries that will be used on all pages in your site.

Source: https://processwire.com/blog/posts/processwire-2.6.7-core-updates-and-more/

 

So... in your case it seems as it's called and used almost every time a page loads/renders.

[a-ok]

22 minutes ago, wbmnfktr said:

Source: https://processwire.com/blog/posts/processwire-2.6.7-core-updates-and-more/

 

So... in your case it seems as it's called and used almost every time a page loads/renders.

Which is bizarre, right? Because if it's an `addHookMethod()` the docs say 'This enables you to add a new accessible public method to an existing object, which will execute your hook implementation method when called upon' (source: https://processwire.com/api/ref/wire/add-hook-method/) so why would ready.php run it and not just 'have it ready'?

Is there an alternative way?

[wbmnfktr]

Ok... sorry... I guess I wasn't accurate enough. 

In my understanding the hook is fine. As it is ready to get called whenever the API is ready - which is almost every time in your case. No matter if front-end or admin back-end. That's totally fine.

But your function won't be executed until !$session->get("shopifyShopCurrency"). Which is fine too and the exact thing you wanted.

So... yes... it's bizarre that you already reached your limits. Maybe. I don't know the API limits or the traffic your site gets. Maybe there are lots and lots of bots/spiders on your site that trigger your hook. Or there is enough regular traffic and you need more API requests.

Can you log each request/function call to count how often it's executed?

[psy]

There's no 'escape clause' in your code so it will be triggered on every page view, eg:

wire()->addHookMethod('Page::ipapiGetUserLocation', function($event) use($ipapiHttp, $ipapiAPIKey) {
	$page = $event->object;
	if ($page->template != 'templatename') return;
	// your code here ...
}

 

[adrian]

Why not cache the user's location in a session variable so you only need to make the API call once per session or with a cookie so it's potentially even less often?

[a-ok]

9 hours ago, psy said:

There's no 'escape clause' in your code so it will be triggered on every page view, eg:

wire()->addHookMethod('Page::ipapiGetUserLocation', function($event) use($ipapiHttp, $ipapiAPIKey) {
	$page = $event->object;
	if ($page->template != 'templatename') return;
	// your code here ...
}

 

The hook is included in the head.inc so it’s not ‘template’ bound – it should call on every template. The issue is it shouldn’t call on every ready.php load – it should only call when the hook is executed.

[a-ok]

7 hours ago, adrian said:

Why not cache the user's location in a session variable so you only need to make the API call once per session or with a cookie so it's potentially even less often?

It should be, no? If `$session->get(“shopifyShopCurrency”)` doesn’t exist then it stores the ipAPI call in it so it should skip the API call next page visit within the session...

[adrian]

6 hours ago, a-ok said:

It should be, no? If `$session->get(“shopifyShopCurrency”)` doesn’t exist then it stores the ipAPI call in it so it should skip the API call next page visit within the session...

Sorry, yes it should - I didn't read the code in your head.inc - I assumed an session setting and checking would have been in the hook.

6 hours ago, a-ok said:

The issue is it shouldn’t call on every ready.php load – it should only call when the hook is executed.

Are you sure it is being called on every ready load? I think you need to do some simple debugging and log or do a bd('called') type check inside the hook and load pages on your site and see what happens.

[bernhard]

18 hours ago, a-ok said:

And in head.inc I'm calling the hook method:

if (!$session->get("shopifyShopCurrency")) {
	if ($page->ipapiGetUserLocation()['currency']['code'] && $page->ipapiGetUserLocation()['currency']['code'] !== 'GBP') {
		$session->set("shopifyShopCurrency", "ROW");
	} elseif (!$session->get("shopifyShopCurrency")) { // Set it as the Shopify store default
		$session->set("shopifyShopCurrency", $shopifyShopCurrency);
	}
} else {
	$session->set("shopifyShopCurrency", $shopifyShopCurrency);
}

Adding the hook like you did is perfectly fine. The problem must be in your head.inc that most likely calls the ipapiGetUserLocation() on every page load

Quote

Are you sure it is being called on every ready load? I think you need to do some simple debugging and log or do a bd('called') type check inside the hook and load pages on your site and see what happens.

+1

[adrian]

BTW, if you are hitting limits with that service, maybe try https://ip.nf/ or https://api.ip.sb/geoip - the former guys are very supportive of PW.

[a-ok]

2 minutes ago, bernhard said:

Adding the hook like you did is perfectly fine. The problem must be in your head.inc that most likely calls the ipapiGetUserLocation() on every page load

Okay thanks. So can we confirm that even though ready.php is called on every page load (back and front) it will only run the hook when told to in my templates? It won't run when someone is on the backend for example?

[bernhard]

2 hours ago, a-ok said:

Okay thanks. So can we confirm that even though ready.php is called on every page load (back and front) it will only run the hook when told to in my templates? It won't run when someone is on the backend for example?

Yes. You can easily check that with tracy debugger:

// ready.php
$wire->addHook("Page::foo", function(Hookevent $event) {
	bd("foo was fired!");
});

You will not see any dumps of "foo was fired!" unless you explicitly call it somewhere else, eg $page->foo() in one of your templates.

[teppo]

On 2/25/2019 at 10:10 PM, a-ok said:

And in head.inc I'm calling the hook method:

if (!$session->get("shopifyShopCurrency")) {
	if ($page->ipapiGetUserLocation()['currency']['code'] && $page->ipapiGetUserLocation()['currency']['code'] !== 'GBP') {
		$session->set("shopifyShopCurrency", "ROW");
	} elseif (!$session->get("shopifyShopCurrency")) { // Set it as the Shopify store default
		$session->set("shopifyShopCurrency", $shopifyShopCurrency);
	}
} else {
	$session->set("shopifyShopCurrency", $shopifyShopCurrency);
}

 

Your hook method isn't doing any caching, so every time it's executed, it makes a request to the API. In your head.inc file above you're checking if shopifyShopCurrency is already stored in session – but if it isn't, you're making two separate requests to the API:

	if ($page->ipapiGetUserLocation()['currency']['code'] && $page->ipapiGetUserLocation()['currency']['code'] !== 'GBP') {

So, it seems to me that simply by grabbing $page->ipapiGetUserLocation() to a variable and using that for those checks you could probably cut your requests for new users to half.

	} elseif (!$session->get("shopifyShopCurrency")) { // Set it as the Shopify store default

Just for the record, this row is kind of odd, since there's no chance that $session->get("shopifyShopCurrency") would be set by now. Although it should work, it's unnecessarily verbose and does kind of a pointless request to $session – a simple "} else {" would've been enough.

It's probably unrelated, but it's also a bit strange that you're querying the currency code based on user location, but you're never actually using it. I'm assuming that this is by design, but it's still kind of curious ?

[a-ok]

4 hours ago, teppo said:

So, it seems to me that simply by grabbing $page->ipapiGetUserLocation() to a variable and using that for those checks you could probably cut your requests for new users to half.

Thanks for the lengthy reply! Yes I guess you're right! Storing it in a $session variable is sort of caching it (so if the user is in session then it won't need to check the ipAPI again) but I get your point about storing it as a variable initially to prevent the double call if not.

4 hours ago, teppo said:

Just for the record, this row is kind of odd, since there's no chance that $session->get("shopifyShopCurrency") would be set by now. Although it should work, it's unnecessarily verbose and does kind of a pointless request to $session – a simple "} else {" would've been enough.

Totally. I have since removed this after giving it more thought.

4 hours ago, teppo said:

 It's probably unrelated, but it's also a bit strange that you're querying the currency code based on user location, but you're never actually using it. I'm assuming that this is by design, but it's still kind of curious ?

Yes sorry I am using it throughout the templates – this is just my initial check ?

[a-ok]

Just an update. This is my current (updated) code. The `$shopifyShop` stuff is related to the Shopify API.

I have a 'currency select' element which the user can use (initial check).

$shopifyShop = $page->shopifyShop();
$shopifyShopCurrency = $shopifyShop['shop']['currency'];

if ($this->input->post("region")) {

	$session->set("shopifyShopCurrency", $this->input->post("region"));

} else {

	if (!$session->get("shopifyShopCurrency")) {
		$ipapiCurrencyCode = $page->ipapiGetUserLocation()['currency']['code'];
		if ($ipapiCurrencyCode && $ipapiCurrencyCode !== 'GBP') {
			$session->set("shopifyShopCurrency", "ROW");
		} else { // Set it as the Shopify store default
			$session->set("shopifyShopCurrency", $shopifyShopCurrency);
		}
	}
}