Force reload parent product in LinkManagement::addChild() to avoid stale cache

[ddevallan]

Description

LinkManagement::addChild() calls $this->productRepository->get($sku, true) to load the parent configurable product. The 4th parameter $forceReload defaults to false, so if the product is already in the repository's in-memory cache from a prior operation in the same PHP process, the cached (potentially stale) instance is returned.

This is a silent failure in long-lived processes such as queue consumers, where the same ProductRepository instance persists across multiple message handlers. The cache is populated by an earlier operation, and a subsequent addChild() call operates on stale extension attributes.

Most common symptom

StateException: The parent product doesn't have configurable product options.

This error appears even though the configurable product clearly has options in the database — because the cached product object pre-dates when those options were configured.

Why the cache is stale

The ProductRepository maintains an in-memory store keyed by {sku}:{editMode}:{storeId}. addChild() loads with editMode=true. If any prior code in the same process loaded the product with the same editMode=true (e.g., another queue message, an import step, or an earlier API call in the same consumer worker), that cached instance is returned.

Reproduction

The following simulates the queue consumer scenario in a single PHP script:

// Simulates "prior operation" warming the cache
$product = $productRepository->get('Configurable Product 1', true);

// Simulates stale state: options were not yet set when the product was first cached
$product->getExtensionAttributes()->setConfigurableProductOptions([]);

// addChild() gets the corrupted cached object → fails with:
// "The parent product doesn't have configurable product options."
$linkManagement->addChild('Configurable Product 1', 'some-simple-sku');

Before fix: StateException: The parent product doesn't have configurable product options.

After fix: addChild() loads fresh from DB → options are present → proceeds correctly.

Fix

Add true as the 4th argument ($forceReload):

- $product = $this->productRepository->get($sku, true);
+ $product = $this->productRepository->get($sku, true, null, true);

BC impact

None. $forceReload=true only affects the in-memory cache behaviour. The DB query and returned product are identical. No API contracts, method signatures, or observable behaviours change for callers.

Manual testing scenarios

1. In a PHP script bootstrapped with Magento, load a configurable product via productRepository->get($sku, true) to warm the cache
2. Clear its configurable options from the cached object: $product->getExtensionAttributes()->setConfigurableProductOptions([])
3. Call $linkManagement->addChild($sku, $childSku)

Before fix: StateException: The parent product doesn't have configurable product options.
After fix: Proceeds to child attribute validation (correct behaviour — the stale parent check is bypassed)

Test results

Link Management (Magento\ConfigurableProduct\Test\Unit\Model\LinkManagement)
 ✔ Get children
 ✔ Get with non configurable product
 ✔ Add child loads parent with force reload
 ✔ Add child state exception

OK (4 tests, 29 assertions)

Note: testAddChild and testRemoveChild in the existing test file use createPartialMockWithReflection(ProductExtensionInterface::class, ...) which fails under PHPUnit 12 because the interface has 15 unimplemented abstract methods. This is a pre-existing issue unrelated to this change.

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated — N/A
• All automated tests passed successfully (all builds are green)

[marcelmtz]

Request a small change according to phpcs. Apart from it, this should be good.

---

I was concern about performance but looks like it should be okay. Created a php script to run 1000 iterations:

Isolated parent-reload cost (the only thing the fix changes):

• Cached get($sku, true) (without fix): ~0.0008 ms (returns existing object reference)
• ForceReload get($sku, true, null, true) (with fix): ~5.8–6.0 ms (full DB + EAV load)
• Reload delta added by the fix: ~6 ms per call

Full addChild() — WITHOUT fix:

• mean: 181.28 ms
• median: 179.37 ms
• p95: 214.89 ms
• max: 345.78 ms

Full addChild() — WITH fix:

• mean: 184.28 ms
• median: 180.66 ms
• p95: 225.59 ms
• max: 378.08 ms

Click to view PHP performance benchmark script

<?php
/**
 * END-TO-END performance benchmark for PR #269.
 *
 * Answers: how big is the forceReload cost (~5.4 ms) RELATIVE to a full
 * LinkManagement::addChild() call, which also does productRepository->save()
 * (+ reindex)?
 *
 * It measures two things in one run:
 *   1. Isolated reload delta  : get(sku,true) vs get(sku,true,null,true)
 *   2. Real addChild() total  : the ACTUAL on-disk method, end to end, using a
 *                               real variant child. Before each timed call the
 *                               child is detached again (setup, NOT timed) and
 *                               the repo cache is cleaned for functional
 *                               correctness.
 *
 * Everything runs inside a DB transaction that is ALWAYS rolled back, so the
 * catalog is never modified.
 *
 * Usage (inside the container):
 *   php bench_addchild_end_to_end.php                 # 20 addChild iterations
 *   php bench_addchild_end_to_end.php 50              # 50 iterations
 *   php bench_addchild_end_to_end.php 30 "Configurable Product 3"
 */
declare(strict_types=1);

use Magento\Framework\App\Bootstrap;
use Magento\Framework\App\State;
use Magento\Framework\App\ResourceConnection;
use Magento\Catalog\Api\ProductRepositoryInterface;
use Magento\Catalog\Model\ProductRepository;
use Magento\ConfigurableProduct\Api\LinkManagementInterface;
use Magento\ConfigurableProduct\Model\ResourceModel\Product\Type\Configurable as ConfigurableResource;

require __DIR__ . '/app/bootstrap.php';

iterations = (int) (argv[1] ?? 20);
parentSku = argv[2] ?? 'Configurable Product 1';

\$bootstrap = Bootstrap::create(BP, \(_SERVER);\)om = \(bootstrap->getObjectManager();\)om->get(State::class)->setAreaCode('adminhtml');

/** @var ProductRepository \$repo */
repo = om->get(ProductRepositoryInterface::class);
/** @var LinkManagementInterface \$linkManagement */
linkManagement = om->get(LinkManagementInterface::class);
/** @var ConfigurableResource \$configurableResource */
configurableResource = om->get(ConfigurableResource::class);

// Detect which version is on disk so the report is labelled correctly.
\(src = file_get_contents(BP . '/app/code/Magento/ConfigurableProduct/Model/LinkManagement.php');\)onDiskFixed = str_contains(src, 'get(sku, true, null, true)');

\$conn = \(om->get(ResourceConnection::class)->getConnection();\)conn->beginTransaction(); // ALWAYS rolled back

\$line = str_repeat('=', 72);

stats = static function (array s): array {
    sort(\$s);
    c = count(s);
    \(sum = array_sum(\)s);
    pct = static fn(float p) => \(s[(int) min(\)c - 1, floor(p * (c - 1)))];
    return ['mean' => sum / c, 'median' => pct(0.5), 'p95' => pct(0.95), 'max' => \(s[\)c - 1]];
};

try {
    // Resolve a real variant child of the parent (has correct attribute values).
    parent = repo->get(\(parentSku);\)childIds = configurableResource->getChildrenIds((int) parent->getId())[0] ?? [];
    if (!\$childIds) {
        throw new RuntimeException("Parent '\$parentSku' has no children to use as a test child.");
    }
    \$childId  = (int) reset(\(childIds);\)childSku = repo->getById(childId)->getSku();

    echo "\$line\n";
    echo "END-TO-END addChild() benchmark (PR #269)\n";
    echo "Parent              : \$parentSku\n";
    echo "Variant child used  : childSku (id=childId)\n";
    echo "addChild iterations : \$iterations\n";
    echo "On-disk version     : " . (\$onDiskFixed ? 'WITH fix (forceReload)' : 'WITHOUT fix (cached get)') . "\n";
    echo "PHP                 : " . PHP_VERSION . "\n";
    echo "\$line\n";

    // ---- Part 1: isolated reload delta -------------------------------------
    warm = 10; micro = 300;
    for (i = 0; i < warm; i++) { repo->get(parentSku, true); repo->get(parentSku, true, null, true); }

    \$t = hrtime(true);
    for (\$i = 0; i < micro; i++) repo->get(\(parentSku, true); }\)cachedAvg = ((hrtime(true) - t) / 1e6) / micro;

    \$t = hrtime(true);
    for (\$i = 0; i < micro; i++) repo->get(\(parentSku, true, null, true); }\)reloadAvg = ((hrtime(true) - t) / 1e6) / micro;

    \$reloadDelta = reloadAvg - cachedAvg;

    // ---- Part 2: full addChild() end to end --------------------------------
    \$samples = [];
    for (i = 0; i < iterations; i++) {
        // SETUP (not timed): make sure the child is detached so addChild re-adds it.
        \$repo->cleanCache();
        try {
            \$linkManagement->removeChild(parentSku, childSku);
        } catch (\Throwable \(e) {             // already detached from a previous iteration's rollback-free loop — fine         }\)repo->cleanCache();

        // TIMED: the real addChild() (get + get + getChildrenIds + options + save + reindex)
        \(start = hrtime(true);\)linkManagement->addChild(parentSku, childSku);
        \(samples[] = (hrtime(true) -\)start) / 1e6;
    }

    \$a = stats(samples);

    echo "\n--- Part 1: isolated parent reload cost --------------------------------\n";
    printf("  cached      get(sku,true)            : %8.4f ms\n", \$cachedAvg);
    printf("  forceReload get(sku,true,null,true)  : %8.4f ms\n", \$reloadAvg);
    printf("  RELOAD DELTA (what the fix adds)     : %8.4f ms\n", \$reloadDelta);

    echo "\n--- Part 2: full addChild() (real method, incl. save + reindex) -------\n";
    printf("  mean   : %8.2f ms\n", \$a['mean']);
    printf("  median : %8.2f ms\n", \$a['median']);
    printf("  p95    : %8.2f ms\n", \$a['p95']);
    printf("  max    : %8.2f ms\n", \$a['max']);

    echo "\n--- Verdict: reload cost as a share of a full addChild() --------------\n";
    // Express the fix's reload delta as a fraction of a full addChild().
    // If on-disk is unfixed, the measured total excludes the reload, so the
    // "with fix" total is (measured + delta).
    \$totalWithoutFix = onDiskFixed ? (a['median'] - reloadDelta) : a['median'];
    totalWithFix = onDiskFixed ? \(a['median'] : (\)a['median'] + \(reloadDelta);\)pct = reloadDelta / max(totalWithFix, 0.0001) * 100;
    printf("  addChild WITHOUT fix (est.)  : %8.2f ms\n", \$totalWithoutFix);
    printf("  addChild WITH fix    (est.)  : %8.2f ms\n", \$totalWithFix);
    printf("  >> the fix adds %.1f%% to a full addChild() call\n", \$pct);
    echo "\$line\n";
} catch (\Throwable \$t) {
    echo "ERROR: " . get_class(t) . ": " . t->getMessage() . "\n";
} finally {
    \(conn->rollBack();\)repo->cleanCache();
    echo "(transaction rolled back — no catalog data was modified)\n";
}

[marcelmtz]

There's a phpcs issue on this line, I can fix it during the next week, but just mentioning if you have a chance before.