Skip to content

Commit 015d930

Browse files
committed
refactor(features): resolve legacy licenses once per feature collection
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.
1 parent 8cacfe2 commit 015d930

3 files changed

Lines changed: 80 additions & 11 deletions

File tree

src/Harbor/Features/Resolve_Feature_Collection.php

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use LiquidWeb\Harbor\Features\Types\Plugin;
1111
use LiquidWeb\Harbor\Features\Types\Service;
1212
use LiquidWeb\Harbor\Features\Types\Theme;
13+
use LiquidWeb\Harbor\Legacy\Legacy_License;
1314
use LiquidWeb\Harbor\Legacy\License_Repository as Legacy_License_Repository;
1415
use LiquidWeb\Harbor\Licensing\Enums\Validation_Status;
1516
use LiquidWeb\Harbor\Licensing\Error_Code as Licensing_Error_Code;
@@ -158,7 +159,8 @@ public function __invoke() {
158159
}
159160
}
160161

161-
$collection = new Feature_Collection();
162+
$collection = new Feature_Collection();
163+
$legacy_licenses = $this->build_legacy_license_map();
162164

163165
foreach ( $catalog as $product ) {
164166
if ( ! $product instanceof Product_Catalog ) {
@@ -169,7 +171,8 @@ public function __invoke() {
169171
$license_tier_rank = $this->resolve_license_tier_rank( $product, $products );
170172

171173
foreach ( $product->get_features() as $catalog_feature ) {
172-
$feature = $this->hydrate_feature( $catalog_feature, $product, $capabilities, $license_tier_rank );
174+
$legacy_license = $legacy_licenses[ $catalog_feature->get_slug() ] ?? null;
175+
$feature = $this->hydrate_feature( $catalog_feature, $product, $capabilities, $license_tier_rank, $legacy_license );
173176

174177
if ( is_wp_error( $feature ) ) {
175178
static::debug_log( $feature->get_error_message() );
@@ -183,6 +186,28 @@ public function __invoke() {
183186
return $collection;
184187
}
185188

189+
/**
190+
* Builds a `slug => Legacy_License` map from the legacy repository.
191+
*
192+
* Resolved once per `__invoke()` call so the `lw-harbor/legacy_licenses`
193+
* filter dispatches once per resolution rather than once per catalog
194+
* feature. This also narrows the surface for filter re-entry: only one
195+
* dispatch can be in-flight while hydrate_feature() iterates.
196+
*
197+
* @since TBD
198+
*
199+
* @return array<string, Legacy_License>
200+
*/
201+
private function build_legacy_license_map(): array {
202+
$map = [];
203+
204+
foreach ( $this->legacy_repository->all() as $license ) {
205+
$map[ $license->slug ] = $license;
206+
}
207+
208+
return $map;
209+
}
210+
186211
/**
187212
* Resolves the capabilities granted by the license for a given product.
188213
*
@@ -271,18 +296,20 @@ private function is_license_invalid( ?string $validation_status ): bool {
271296
*
272297
* @since 1.0.0
273298
*
274-
* @param Catalog_Feature $catalog_feature The catalog feature entry.
275-
* @param Product_Catalog $product The parent catalog product.
276-
* @param string[]|null $capabilities The license capabilities, or null if unlicensed.
277-
* @param int $license_tier_rank The user's licensed tier rank, or -1 if unlicensed.
299+
* @param Catalog_Feature $catalog_feature The catalog feature entry.
300+
* @param Product_Catalog $product The parent catalog product.
301+
* @param string[]|null $capabilities The license capabilities, or null if unlicensed.
302+
* @param int $license_tier_rank The user's licensed tier rank, or -1 if unlicensed.
303+
* @param Legacy_License|null $legacy_license The legacy license entry whose slug matches this feature, or null if none.
278304
*
279305
* @return Feature|WP_Error The hydrated feature, or WP_Error for unknown types.
280306
*/
281307
private function hydrate_feature(
282308
Catalog_Feature $catalog_feature,
283309
Product_Catalog $product,
284310
?array $capabilities,
285-
int $license_tier_rank
311+
int $license_tier_rank,
312+
?Legacy_License $legacy_license
286313
) {
287314
$catalog_kind = $catalog_feature->get_kind();
288315
$class = $this->type_map[ $catalog_kind ] ?? null;
@@ -301,7 +328,6 @@ private function hydrate_feature(
301328
$minimum_tier = $product->get_tier_by_slug( $catalog_feature->get_minimum_tier() );
302329
$minimum_rank = $minimum_tier !== null ? $minimum_tier->get_rank() : PHP_INT_MAX;
303330

304-
$legacy_license = $this->legacy_repository->find( $catalog_feature->get_slug() );
305331
$has_legacy_grant = $legacy_license !== null
306332
&& $legacy_license->is_active
307333
&& $legacy_license->key !== ''

tests/wpunit/Features/Feature_RepositoryTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ public function test_hydrate_feature_returns_wp_error_for_unknown_type(): void {
342342
$method = new ReflectionMethod( Resolve_Feature_Collection::class, 'hydrate_feature' );
343343
$method->setAccessible( true ); // Required for PHP < 8.1.
344344

345-
$result = $method->invoke( $resolver, $catalog_feature, $product, null, -1 );
345+
$result = $method->invoke( $resolver, $catalog_feature, $product, null, -1, null );
346346

347347
$this->assertInstanceOf( WP_Error::class, $result );
348348
$this->assertSame( Error_Code::UNKNOWN_FEATURE_TYPE, $result->get_error_code() );
@@ -388,7 +388,7 @@ public function test_hydrate_feature_returns_feature_for_known_type(): void {
388388
$method = new ReflectionMethod( Resolve_Feature_Collection::class, 'hydrate_feature' );
389389
$method->setAccessible( true ); // Required for PHP < 8.1.
390390

391-
$result = $method->invoke( $resolver, $catalog_feature, $product, [ 'test-plugin' ], 1 );
391+
$result = $method->invoke( $resolver, $catalog_feature, $product, [ 'test-plugin' ], 1, null );
392392

393393
$this->assertInstanceOf( Plugin::class, $result );
394394
$this->assertSame( 'test-plugin', $result->get_slug() );
@@ -445,7 +445,7 @@ public function test_free_tier_feature_available_for_licensed_user_regardless_of
445445

446446
// Omit the free feature from capabilities, simulating a Commerce Portal that only
447447
// lists paid features. The resolver must still mark it available and in tier.
448-
$result = $method->invoke( $resolver, $catalog_feature, $product, [ 'some-paid-feature' ], 1 );
448+
$result = $method->invoke( $resolver, $catalog_feature, $product, [ 'some-paid-feature' ], 1, null );
449449

450450
$this->assertInstanceOf( Plugin::class, $result );
451451
$this->assertTrue( $result->is_available(), 'Free-tier feature must be available regardless of capabilities.' );

tests/wpunit/Features/Resolve_Feature_CollectionTest.php

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,4 +414,47 @@ public function test_resolver_without_legacy_repo_argument_uses_filter_backed_de
414414

415415
$this->assert_resolved_feature_flags( $resolver, 'kad-blocks-pro', true, true );
416416
}
417+
418+
/**
419+
* Tests that resolving a catalog with multiple features dispatches the
420+
* `lw-harbor/legacy_licenses` filter exactly once.
421+
*
422+
* Before the legacy lookup was hoisted out of hydrate_feature(), the filter
423+
* fired once per catalog feature. That meant every consumer hooked to the
424+
* filter ran N times on every resolution, and any callback that consults
425+
* Harbor itself (e.g. Solid Backups via lw_harbor_is_feature_available)
426+
* compounded that cost N-fold. Hoisting the lookup to once per __invoke()
427+
* is what this test pins down.
428+
*
429+
* @return void
430+
*/
431+
public function test_legacy_filter_dispatches_once_per_resolution_not_per_feature(): void {
432+
$dispatch_count = 0;
433+
434+
add_filter(
435+
'lw-harbor/legacy_licenses',
436+
static function ( array $licenses ) use ( &$dispatch_count ) {
437+
++$dispatch_count;
438+
439+
return $licenses;
440+
}
441+
);
442+
443+
$resolver = $this->make_resolver( $this->make_catalog(), new Product_Collection() );
444+
445+
$collection = ( $resolver )();
446+
447+
$this->assertInstanceOf( Feature_Collection::class, $collection );
448+
$this->assertGreaterThan(
449+
1,
450+
$collection->count(),
451+
'Sanity: the fixture catalog must contain more than one feature for this test to be meaningful.'
452+
);
453+
$this->assertSame(
454+
1,
455+
$dispatch_count,
456+
'The lw-harbor/legacy_licenses filter must fire exactly once per resolution, '
457+
. 'not once per catalog feature.'
458+
);
459+
}
417460
}

0 commit comments

Comments
 (0)