feat: pass legacy license keys to Herald for updates and downloads#159
Conversation
Lets customers with legacy per-plugin license keys (reported via the lw-harbor/legacy_licenses filter) receive update checks and download their packages via Herald's /legacy/download endpoint, even without a Unified key. Active legacy entries grant catalog feature availability for their matching slug and take precedence over Unified URLs in Herald_Url_Builder. The legacy key must be non-empty and is_active must be true for the grant to apply.
The lw-harbor/legacy_licenses filter contract requires both key and slug. Entries missing either field were previously cached and surfaced inconsistently (UI/notices showed them while access paths defensively rejected them). Drop them in License_Repository::all() so they never reach any downstream consumer.
Document how reported legacy keys feed into Resolve_Feature_Collection (availability grant), Plugin_Handler/Theme_Handler (update checks), and Herald_Url_Builder (download URL with precedence over Unified). Also clarify the semantics of is_active and what happens to malformed entries.
PHPUnit only picks up methods prefixed with test_. The it_* prefix is the Codeception .cest format, so the 11 existing methods in this file were silently skipped by the wpunit suite. Renaming them brings 11 previously-dormant tests into the suite (37 new assertions, all passing).
…rder Resolution prefers Unified because it is the canonical source of entitlement; URL building prefers Legacy because legacy keys are scoped to a specific slug while the Unified key only authenticates features in its capabilities array (which the URL builder does not consult). Preferring legacy when present avoids generating Unified URLs that Herald would reject for slugs the Unified tier does not include but a legacy add-on does.
…ity branches Promote $has_legacy_grant into the unconditional-true branch alongside wporg and free-tier, then collapse the remaining elseif/else into a single else guarded by $capabilities !== null short-circuit. The null guard the original elseif provided against in_array() is preserved.
Both fields are typed strings, so the empty-string comparison and the truthiness check are equivalent. The tighter form reads better and was the form requested in review.
build() now decides which license type covers the slug; the legacy and Unified URL string assembly each live in their own private helper, and both go through a single herald_url() composer that owns the base-URL concatenation and add_query_arg() call. The public Download_Url_Builder contract is unchanged.
Site\Data::get_domain() returns string today, so the existing === '' check is functionally identical. Swapping to ! $domain future-proofs the guard against the return type ever loosening to ?string.
…plitting legacy out The earlier change added Legacy_License_Repository as a required third argument to Herald_Url_Builder, altering the class's expected constructor shape and breaking the public contract for any caller that instantiates it directly. Restore the original Herald_Url_Builder( License_Repository, Data ) signature and move the new behavior into sibling classes: - Herald_Url_Builder: Unified license URL only -- original 2-arg signature preserved verbatim from main - Herald_Legacy_Url_Builder: legacy license URL only - Herald_Routing_Url_Builder: implements Download_Url_Builder, holds both and returns the first non-empty result (legacy first, Unified fallback) Portal Provider now binds Download_Url_Builder to Herald_Routing_Url_Builder, so consumers of the contract get the same end-to-end behavior the previous design provided, without Herald_Url_Builder itself being touched.
…nt optional Resolve_Feature_Collection previously required Legacy_License_Repository as a fourth constructor argument, breaking the original three-argument shape any caller would have been relying on. Make the new argument nullable with a null default and lazily build a fresh Legacy_License_Repository when omitted. Both the container-managed singleton and the default fresh instance read the same `lw-harbor/legacy_licenses` filter, so callers using the three-arg form still see legacy entries registered elsewhere in the request. A new test instantiates the resolver without the optional argument and asserts that a filter-registered legacy entry still grants availability, guarding the compatibility shim against future regressions.
…branch Eight new methods in Resolve_Feature_CollectionTest and one new method in License_RepositoryTest (test_drops_entries_with_empty_key_or_slug) were introduced without the project's PHPDoc convention. Each now leads with a "Tests..." sentence and declares @return void, matching the form used elsewhere in the suite. Pre-existing tests on main are left untouched.
- Add `rawurlencoded` to the inline cspell ignore directives in both Herald_Url_BuilderTest and Herald_Legacy_Url_BuilderTest. The earlier directive only ignored `rawurlencodes` (method-name form); the new test docblocks use the past-tense form. - Realign the Herald URL builder table in docs/subsystems/portal.md so pipes line up under the header, satisfying markdownlint MD060.
Adds a per-entry `use_for_updates` flag (default false) to the `lw-harbor/legacy_licenses` filter payload. Only opt-in entries grant catalog feature availability, contribute a Herald `/legacy/download` URL, and trigger Plugin/Theme update handlers. Non-opt-in entries continue to appear in the licensing UI and admin notices. Protects Harbor from advertising "update available" badges for legacy keys whose backend is not Stellar Licensing v3 compatible (e.g. SolidWP API keys, some Give legacy keys), which would otherwise fail validation when the user clicks Update. Addresses PR #159 review feedback.
|
I'm trying to test, but got: It looks like an endless loop happens, will check. |
AI slopWhy it didn't loop before this branchThe regression was introduced by commit 5dd88c0 feat: pass legacy license keys to Herald for updates and downloads (Eric Defore, 2026-05-15). The structural change Before 5dd88c0, Resolve_Feature_Collection::hydrate_feature() only consulted:
After 5dd88c0, hydration got a new collaborator — Legacy_License_Repository — and now does this for every feature being resolved (Resolve_Feature_Collection.php:302-306): $legacy_license = $this->legacy_repository->find( $catalog_feature->get_slug() ); Why that closes the loop Legacy_License_Repository::find() → ::all() → apply_filters( 'lw-harbor/legacy_licenses', … ) (License_Repository.php:32). That filter has had a closure registered by Solid Backups' updater since forever (init.php:189-195) which calls Ithemes_Updater_Harbor::get_legacy_licenses() → Ithemes_Updater_Packages::get_full_details() → Ithemes_Updater_API::get_request_package_details() → Ithemes_Updater_Harbor::is_product_managed() Before 5dd88c0, the chain stopped at lw_harbor_is_feature_available because feature resolution never re-entered the legacy repository. After 5dd88c0, it does — and the recursion closes. Summary ┌───────────────────────────────────────────────────────────────────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ Before 5dd88c0, the chain stopped at lw_harbor_is_feature_available because feature resolution never re-entered the legacy repository. After 5dd88c0, it does — and the recursion closes. Summary ┌───────────────────────────────────────────────────────────────────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ The two follow-ups on the branch (73f2c19, cf739c3) tweaked the legacy-grant logic but didn't change the fundamental coupling — the moment hydrate_feature() started calling legacy_repository->find(), anything wired into the lw-harbor/legacy_licenses filter became reachable from inside Harbor's own Where to fix You have a few choices, in roughly increasing invasiveness:
Option 1 or 3 belongs in Harbor; option 2 belongs in Solid Backups. If you control both, I'd lean toward 3 — calling apply_filters once per feature is wasteful even when it doesn't recurse. So the issue is inside the |
|
@johnhooks @shvlv Weird... I haven't seen this issue in any of my testing :| I'll investigate more closely in a bit. |
|
I don't know what the contract is. If we can report from the plugin side all legacy licenses we have, regardless of whether they are managed by Harbor as well, we can do it for sure. Because I see other plugins didn't do that - https://github.com/stellarwp/kadence-shop-kit/blob/515ba5033a7266e4f3cc1f29642d2387cd2b5a77/inc/Common/Harbor/Legacy_License_Integration.php#L89-L115. So we can simplify our logic and remove that check too https://github.com/ithemes/updater/blob/master/harbor.php#L133-L135 🤔 |
Solid Backups' lw-harbor/legacy_licenses callback consults lw_harbor_is_feature_available, which since 5dd88c0 routes through feature resolution back into License_Repository::all() before the outer apply_filters dispatch has completed. WordPress does not detect a hook re-dispatching itself, so WP_Hook re-fires every registered callback at a new nesting level until the PHP call stack is exhausted. Add a per-instance applying_filter flag wrapped around the apply_filters call so re-entrant calls during dispatch return an empty array and break the cycle. Re-entrant callers are answered from Unified data alone, which is the only consistent source while dispatch is in progress.
Previously hydrate_feature() called legacy_repository->find() once per catalog feature, which dispatched the lw-harbor/legacy_licenses filter on every iteration. Beyond the wasted work that hurt scaling with catalog size, it widened the surface for re-entrant filter callbacks: any callback consulting Harbor itself (e.g. Solid Backups via lw_harbor_is_feature_available) compounded that cost per feature. Hoist the lookup to __invoke(), build a slug => Legacy_License map once, and pass each feature's match (or null) into hydrate_feature(). Filter dispatches drop from O(features) to O(1) per resolution, and any re-entry guard in License_Repository now only has to cover a single dispatch window. Existing reflection-based hydrate_feature() tests in Feature_RepositoryTest pass the new fifth argument as null since they exercise paths that don't touch the legacy grant.
The previous docblock framed the test as a regression guard against a
prior behavior ("Before the legacy lookup was hoisted out of
hydrate_feature()..."), but the per-feature lookup it described was
only ever present in-progress on this feature branch and never shipped.
Rephrase as forward-looking design intent: the resolver must dispatch
the filter once per __invoke() and any naive per-feature implementation
is what this test prevents.
|
@johnhooks @shvlv I'm trying to ensure I can reproduce the issue with iThemes Updater on my end, but I think this should address the issue within Harbor in a way that won't require an update to iThemes Updater. Edit: I was able to reproduce the issue. Turns out my |
shvlv
left a comment
shvlv
left a comment
There was a problem hiding this comment.
It works well with iThemes updater!
The single question I have is about UI. Should we update this notice?
Or legacy users should not see that at all because of the new Welcome screen, and they can update from the WordPress update screen (while the new unified key might not have the Kadence tier in theory).
@shvlv Yeah, I found that notice strange 🤔 Not being allowed to update from this screen is one thing, but the text is misleading. Maybe
? They should still be getting updates and support with a Legacy License. |
|
@d4mation that works for me
|
Maybe we should call this place somehow 😂 Software License Manager? |
They _do_ get updates and support with a Legacy License
d4mation
deleted the
feature/utilize-registered-legacy-licenses-for-updates
branch
4 participants
Summary
Harbor now forwards per-plugin Legacy License Keys (registered via the
lw-harbor/legacy_licensesfilter, whether sourced from Uplink or any custom legacy storage) to Herald's/legacy/downloadendpoint, and treats an active legacy entry as an availability + in-tier grant for its matching catalog slug. This lets a customer with only a legacy key receive update notifications and download packages directly through Harbor, without a Unified key.Why this matters
This unblocks decommissioning ZIP uploads to the legacy licensing servers: as long as Herald accepts the key on
/legacy/download, Harbor can take it the rest of the way.A simple Cloudflare-style silent redirect from the legacy licensing server's download URL to Herald isn't sufficient on its own, because update discoverability still needs to happen on the WordPress side. Catalog version metadata flows through Portal, the resolver, and the update handlers; only with that local pipeline aware of legacy entitlements does the customer ever see "an update is available" in the first place. Doing this in Harbor turns out to be the simplest path forward.
IMPORTANT
Right now, sending a Legacy License to Harbor to download Shop Kit doesn't work because of this: https://lw.slack.com/archives/C0ADJGSME5Q/p1778730684475449
We will want to wait until https://linear.app/nexcess/issue/CONS-229/re-introduce-uplink-into-shop-kit-pro is complete before deploying this change.
📹 https://www.loom.com/share/2cdde6cddbb44a75989920ec079b5bee