[4.x] Correct DomainTenantResolver::isSubdomain() check

[lukinovec]

Added a failing test for determining if a host is a subdomain, then fixed DomainTenantResolver::isSubdomain() (same fix as in #1423) and a related assertion.

Previously, while having tenancy.identification.central_domains set to e.g. ['site.com'], the isSubdomain() check consider tenantsite.com a subdomain because it ends with site.com. Now, instead of the endsWith() check, the method checks if the passed domain is in the configured central domains. If it is, it returns false. Otherwise, loop through all the central domains and check if the passed domain matches any of the central domains prefixed with a dot (e.g. tenant.site.com would be considered a subdomain, tenant.site.com wouldn't).

Summary by CodeRabbit

• Bug Fixes
  • Fixed domain identification so central domains are not misclassified as tenant subdomains and multiple central domains are handled correctly.
• Tests
  • Added and adjusted tests to verify subdomain detection and updated expectations for domain-based identification behavior.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.06%. Comparing base (c32f52c) to head (7426adf).

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1425      +/-   ##
============================================
+ Coverage     86.03%   86.06%   +0.02%     
- Complexity     1156     1159       +3     
============================================
  Files           184      184              
  Lines          3381     3387       +6     
============================================
+ Hits           2909     2915       +6     
  Misses          472      472              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kostamilorava]

#1398 is resolved here. I will close my PR #1423, which was used to make this one.

[coderabbitai]

📝 Walkthrough

Walkthrough

DomainTenantResolver::isSubdomain() was changed to normalize configured central domains into an array, explicitly exclude exact central-domain matches, and detect subdomains by checking string suffixes; tests were updated and a new unit test added to verify the behavior.

Changes

Cohort / File(s)	Summary
Domain Subdomain Resolution src/Resolvers/DomainTenantResolver.php	Normalize tenancy.identification.central_domains via Arr::wrap(), perform exact-match exclusion using strict in_array, and iterate central domains to check subdomain suffixes (requires . + central domain).
Tests — Early Identification tests/EarlyIdentificationTest.php	Updated expected exception for a central-route domain from NotASubdomainException to TenantCouldNotBeIdentifiedOnDomainException.
Tests — Subdomain Detection tests/SubdomainTest.php	Added import for DomainTenantResolver and a new test asserting isSubdomain() returns false for exact central domains, true for proper subdomains, and false for non-matching domains (site.com, blog.site.com, tenant.site.com, tenantsite.com).

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I sniff the domains both near and far,
Exact ones sit still, subdomains hop like stars,
Wrapped in arrays, I end with a dot,
No false positives—just the right spot. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title clearly and concisely identifies the main change: correcting the DomainTenantResolver::isSubdomain() method to properly handle domain subdomain detection.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch subdomain-check

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Resolvers/DomainTenantResolver.php`:
- Around line 64-71: The loop in DomainTenantResolver that checks centralDomains
currently does suffix checks before ensuring longer/explicit central domains are
matched, causing order-dependent misclassification (e.g., 'test' vs 'app.test').
Fix by ensuring exact or more specific central domain matches are evaluated
first: either pre-check for any exact match of $domain against $centralDomains
(return false) before doing Str::endsWith checks, or sort $centralDomains by
descending length (so 'app.test' is checked before 'test') and then keep the
existing Str::endsWith logic; update the foreach over $centralDomains
accordingly in the resolver.

In `@tests/SubdomainTest.php`:
- Around line 112-117: Add assertions covering the exact central-domain and
overlapping central-domain cases: keep the existing
config(['tenancy.identification.central_domains' => ['app.test']]) test but also
assert DomainTenantResolver::isSubdomain('app.test') returns false (exact match
should not be treated as subdomain); then add a case with
config(['tenancy.identification.central_domains' => ['test','app.test']]) and
assert DomainTenantResolver::isSubdomain('foo.app.test') is true while
DomainTenantResolver::isSubdomain('app.test') remains false to ensure
overlapping central domains are handled correctly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e3ca179b-6f2f-4716-a29e-2afdbd59a2a4

📥 Commits

Reviewing files that changed from the base of the PR and between 60dd522 and 3976880.

📒 Files selected for processing (3)

• src/Resolvers/DomainTenantResolver.php
• tests/EarlyIdentificationTest.php
• tests/SubdomainTest.php

[Copilot]

Pull request overview

This PR fixes subdomain detection in DomainTenantResolver::isSubdomain() so that central domains aren’t misclassified as tenant subdomains (e.g. tenantsite.com no longer matches site.com just by suffix), and it updates/extends tests to lock in the corrected behavior.

Changes:

• Added a regression test for DomainTenantResolver::isSubdomain() covering central-domain exact matches and dot-boundary subdomains.
• Updated DomainTenantResolver::isSubdomain() to (1) reject exact central-domain matches and (2) only treat *.{centralDomain} as subdomains.
• Adjusted an early-identification test expectation for InitializeTenancyByDomainOrSubdomain to match the new behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
tests/SubdomainTest.php	Adds a regression test for corrected subdomain detection with multiple central domains.
tests/EarlyIdentificationTest.php	Updates expected exception type for domain-or-subdomain middleware when hitting a central route under tenant-default routing.
src/Resolvers/DomainTenantResolver.php	Fixes isSubdomain() to use exact-match exclusion + dot-suffix matching against configured central domains.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.