Skip to content

Drop the unused token-factory wiring from CaptureAction - #50

Merged
loevgaard merged 1 commit into
2.xfrom
fix/issue-3-unused-token-factory
Aug 10, 2026
Merged

Drop the unused token-factory wiring from CaptureAction#50
loevgaard merged 1 commit into
2.xfrom
fix/issue-3-unused-token-factory

Conversation

@loevgaard

Copy link
Copy Markdown
Member

Refs #3.

CaptureAction implemented GenericTokenFactoryAwareInterface and used the trait, but never touched
$this->tokenFactory — a capture needs no token. Only AuthorizeAction mints one, for the callback url.

That was dragging payum/core's deprecated GenericTokenFactoryInterface into the class for nothing, so
removing it halves this package's exposure to the deprecation. The test now pins the intent from the
other side — asserting the action does not implement the interface — so it cannot drift back in.

No behaviour change: Payum's GenericTokenFactoryExtension simply stops injecting a factory that was
never read.

Why #3 cannot be fully fixed

The remaining use in AuthorizeAction is not removable, and the deprecation is inert:

  • What we implement is not deprecated. GenericTokenFactoryAwareInterface and
    GenericTokenFactoryAwareTrait carry no deprecation. Only GenericTokenFactoryInterface does —
    "deprecated since 1.3.7 and will be removed in 2.0".
  • payum/core 2.0 does not exist. The installed version is 1.7.7, released years after that
    annotation, and our constraint is payum/core: ^1.6, so a hypothetical removal cannot reach us
    silently.
  • payum/core still depends on it itself. Payum::__construct() takes a GenericTokenFactoryInterface
    and Payum::getTokenFactory() returns one. It cannot go without a major release of payum/core.
  • There is no replacement for what we need. AuthorizeAction calls createNotifyToken() to build
    the callback url. The non-deprecated TokenFactoryInterface::createToken() requires the caller to
    supply the notify path — but that path is consumer-configured via setGenericTokenFactoryPaths()
    (Payum's own default is notify.php; our e2e harness uses notify). Hardcoding it would break
    consumers, which is worse than the deprecation.

So the practical answer is: use what payum/core still ships and still wires, keep the exposure minimal,
and revisit if payum/core ever tags a 2.0.

CaptureAction implemented GenericTokenFactoryAwareInterface and used the trait,
but never touched $this->tokenFactory — a capture needs no token. Only
AuthorizeAction mints one, for the callback url.

That was pulling payum/core's deprecated GenericTokenFactoryInterface into the
class for nothing, so removing it halves this package's exposure to the
deprecation in #3. The test now pins the intent from the other side, asserting the
action does NOT implement the interface, so it cannot drift back in.

No behaviour change: Payum's GenericTokenFactoryExtension simply stops injecting a
factory that was never read.

Refs #3
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.40%. Comparing base (993aedf) to head (98824c0).

Additional details and impacted files
@@            Coverage Diff            @@
##                2.x      #50   +/-   ##
=========================================
  Coverage     97.40%   97.40%           
  Complexity      126      126           
=========================================
  Files            14       14           
  Lines           347      347           
=========================================
  Hits            338      338           
  Misses            9        9           

☔ View full report in Codecov by Harness.
📢 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.

@loevgaard
loevgaard merged commit 448abf9 into 2.x Aug 10, 2026
21 checks passed
@loevgaard
loevgaard deleted the fix/issue-3-unused-token-factory branch August 10, 2026 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant