Skip to content

Conversation

@sk757a
Copy link

@sk757a sk757a commented Jan 7, 2026

Description
This PR enhance the framework's capabilities by adding a new caching driver based on the APCu extension.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@mergeable
Copy link

mergeable bot commented Jan 7, 2026

Hi there, sk757a! 👋

Thank you for sending this PR!

We expect the following in all Pull Requests (PRs).

Important

We expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite to prove the code works.

If pull requests do not comply with the above, they will likely be closed. Since we are a team of volunteers, we don't have any more time to work
on the framework than you do. Please make it as painless for your contributions to be included as possible.

See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

Sincerely, the mergeable bot 🤖

@sk757a sk757a changed the title APCu caching driver implementation feat: APCu caching driver Jan 7, 2026
@paulbalandan paulbalandan added enhancement PRs that improve existing functionalities 4.7 labels Jan 8, 2026
@paulbalandan
Copy link
Member

The test failures are due to $this->handler being an instance of DummyHandler. This means the apcu driver's ->isSupported() call in CacheFactory returns false so you may want to debug there why it is not working on Github actions.

@sk757a
Copy link
Author

sk757a commented Jan 8, 2026

The test failures are due to $this->handler being an instance of DummyHandler. This means the apcu driver's ->isSupported() call in CacheFactory returns false so you may want to debug there why it is not working on Github actions.

It seems to me that APCu extension just needs to be installed in the CI environment.

@paulbalandan
Copy link
Member

It seems to me that APCu extension just needs to be installed in the CI environment.

Please add then in test-phpunit.yml under the extra-extensions parameter of cache-live-tests job.

@sk757a
Copy link
Author

sk757a commented Jan 8, 2026

APCu enabled. Can't get why tests failed.

@paulbalandan
Copy link
Member

Probably apcu_enabled() returns false?

@michalsn
Copy link
Member

michalsn commented Jan 8, 2026

By default, APCu is disabled in CLI, see: https://www.php.net/manual/en/apcu.configuration.php#ini.apcu.enable-cli

We can use:

ini-values: apc.enable_cli=1

But this should be abstracted, like other options.

Copy link
Contributor

@datamweb datamweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Please provide full details in the PR description.

@sk757a
Copy link
Author

sk757a commented Jan 8, 2026

By default, APCu is disabled in CLI, see: https://www.php.net/manual/en/apcu.configuration.php#ini.apcu.enable-cli

This. Forgot about this parameter.

We can use:

ini-values: apc.enable_cli=1

But this should be abstracted, like other options.

Could you elaborate a bit? At first glance, I just wanted to add -d apcu.enable_cli=1 at the end of

extra-composer-options: ${{ matrix.composer-option }}

@michalsn
Copy link
Member

michalsn commented Jan 8, 2026

I don't think it will have any effect if you add it to the extra-composer-options.

The proper way to add ini settings to setup-php is by ini-values: https://github.com/marketplace/actions/setup-php-action#ini-values-optional

So I think we should have an abstract value, like: extra-ini-options, which will be applied when we setup php:

- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ inputs.php-version }}
tools: composer
extensions: gd, ${{ inputs.extra-extensions }}
coverage: ${{ env.COVERAGE_DRIVER }}

@sk757a
Copy link
Author

sk757a commented Jan 8, 2026

I don't think it will have any effect if you add it to the extra-composer-options.

I was in a hurry and made a mistake. The composer parameters are of course irrelevant. I rather meant something like this
extra-phpunit-options: -d apcu.enable_cli=1

Thanks for the clarification. I'll look into this now.

@sk757a
Copy link
Author

sk757a commented Jan 8, 2026

Time dependent test fail. May be just rerun?

Copy link
Member

@paulbalandan paulbalandan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add ext-apcu in the suggested extensions in composer.json? Both root composer.json and framework composer.json

description: Additional PHP extensions that are needed to be enabled
type: string
required: false
extra-ini-options:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change to this file should go the develop branch in a separate PR as I believe it can be used early.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should make similar changes to the reusable-serviceless-phpunit-test.yml file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please

Comment on lines +36 to +38
/**
* {@inheritDoc}
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can safely remove the inheritdocs tags here.

/**
* {@inheritDoc}
*/
public function increment(string $key, int $offset = 1): bool|int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For here and decrement, you can change the return type to false|int

/**
* {@inheritDoc}
*/
public function getCacheInfo(): array|false|object|null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can limit the return type to just array|false

Comment on lines +44 to +46
if (! extension_loaded('apcu')) {
$this->markTestSkipped('APCu extension not loaded.');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use a class-level attribute #[\PHPUnit\Framework\Attributes\RequiresPhpExtension('apcu')] instead

@sk757a sk757a requested a review from datamweb January 9, 2026 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.7 enhancement PRs that improve existing functionalities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants