Fix issue 632: cached lookup

[ralfinsh]

Fixes #632

What I changed

https://github.com/stancl/tenancy/blob/27e9fb4a692a15ca2fd320799d97fc657cb14b68/src/Resolvers/DomainTenantResolver.php#L31-L38

Instead of querying Domain model, find Tenant and eager load its domains via Tenant model. This way, you get $tenant, which already has loaded domains relationship , with only the current domain (Tenant can have many domains, but $tenant->domains will have only the identified domain!). This reduces SQL queries and memory usage - when using cache, Domain models are loaded from cache, not from the central database. Now there is no need to access central database, if you need to get current Domain or Tenant models, or any of Tenant's domains.

Added a new method to CachedTenantResolver - tenantIdentifiedFromCache(Tenant $tenant, ...$args), which is called everytime, when Tenant is identified from cache. Then it's used in resolver, to set the current domain.

Added a new method to CachedTenantResolver - tenantIdentified(Tenant $tenant, ...$args), which is used to set $currentDomain in DomainTenantResolver.

[ralfinsh]

I can't reproduce these failures locally with running tests on docker, with PHP 8.0 and trying both Laravel ^6.0 and ^8.0.
Feels like, there is something weird happening on the CI tests.
Could You, @stancl, please, chek it?

[codecov]

Codecov Report

Merging #633 (fe152b7) into 3.x (27e9fb4) will increase coverage by 0.06%.
The diff coverage is 94.44%.

❗ Current head fe152b7 differs from pull request most recent head 597eebd. Consider uploading reports for the commit 597eebd to get more accurate results

@@             Coverage Diff              @@
##                3.x     #633      +/-   ##
============================================
+ Coverage     86.92%   86.98%   +0.06%     
- Complexity      366      369       +3     
============================================
  Files           103      103              
  Lines          1086     1099      +13     
============================================
+ Hits            944      956      +12     
- Misses          142      143       +1     

Impacted Files	Coverage Δ	Complexity Δ
src/Resolvers/Contracts/CachedTenantResolver.php	95.23% <75.00%> (-4.77%)	9.00 <1.00> (+1.00)	⬇️
src/Resolvers/DomainTenantResolver.php	100.00% <100.00%> (ø)	5.00 <2.00> (+2.00)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27e9fb4...597eebd. Read the comment docs.

[ralfinsh]

Now, with this pull request:

• Issue $currentDomain is null when using cached lookup #632 is fixed
• When accessing tenant() or tenant()->domains (also, it shows all domains) with cached lookup enabled, there are no calls made to central db (so this fix does not affect existing functionality)

[stancl]

I made some changes to the code, can you please check what test started failing?

[ralfinsh]

I made some changes to the code, can you please check what test started failing?

@stancl Github Actions sometimes fail for no reason. Thanks for the cleanup! I pushed a commit (to get the tests run again) and now everything passed.

[stancl]

Thanks!

[shimsag]

I'm getting below error after upgrade from 3.4.2 to 3.4.3
Can you please assist ?

SQLSTATE[42883]: Undefined function: 7 ERROR: operator does not exist: character varying = integer LINE 1: select * from "domains" where "domains"."tenant_id" in (0) ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. (SQL: select * from "domains" where "domains"."tenant_id" in (0))

[aman-rohiman]

I'm getting below error after upgrade from 3.4.2 to 3.4.3
Can you please assist ?

SQLSTATE[42883]: Undefined function: 7 ERROR: operator does not exist: character varying = integer LINE 1: select * from "domains" where "domains"."tenant_id" in (0) ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. (SQL: select * from "domains" where "domains"."tenant_id" in (0))

Same to me, I getting error like that after upgrade from 3.4.2 to 3.4.3

[ralfinsh]

@shimsag could you please open an issue with details and steps to reproduce. I think the problem is in your code. Can't assist without details.

[aman-rohiman]

@shimsag could you please open an issue with details and steps to reproduce. I think the problem is in your code. Can't assist without details.

In my case, I'm using Postgres 11, and everything is normal, but after i run composer update, when I try to open tenant domain, I'm getting those error.
Step to reproduce:

1. Use postgres database
2. Open tenant before upgrade, everything went normal
3. Composer update
4. Open tenant after upgrade
5. Error "SQLSTATE[42883]" shown up
   To make sure the source of problem, I replace vendor stancl/tenancy with previous version (3.4.2) everything went normal, but after replace it again with stancl/tenancy 3.4.3, the error show up again.
   I hope it helps.

[stancl]

What's your data type for tenant_id on domains table and the id on the tenants table?

[stancl]

The only change is that the domain is not getting fetched via Domain::find() but via the relation ($tenant->domains()->where()) so the issue must be a mismatching type for those two columns.

[aman-rohiman]

What's your data type for tenant_id on domains table and the id on the tenants table?

Data type for tenant_id column on domains table is varchar(255), for example "cbab4d88-5b45-4c91-8cf7-37accf642960", this is same data type with id column on tenants table, i'm using saas boilerplate and using postgresql database, and this problem occured after composer update.

[stancl]

Can you try running composer update and checking if this is fixed now?

[aman-rohiman]

Can you try running composer update and checking if this is fixed now?

Nice, it works now, thank you @stancl for the fast response.

[stancl]

Great, thanks!