Skip to content

Spark no longer trigger pre_system and post_system events #8442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Martin-4Spaces opened this issue Jan 21, 2024 · 12 comments
Closed

Spark no longer trigger pre_system and post_system events #8442

Martin-4Spaces opened this issue Jan 21, 2024 · 12 comments
Labels
enhancement PRs that improve existing functionalities

Comments

@Martin-4Spaces
Copy link

PHP Version

8.3

CodeIgniter4 Version

4.4.4

CodeIgniter4 Installation Method

Composer (as dependency to an existing project)

Which operating systems have you tested for this bug?

macOS, Linux

Which server did you use?

apache

Database

No response

What happened?

After updating CodeIgniter, some commands broke.
The problems are caused by some code not being executed. Code that fired when events such as pre_system and post_system are triggered.

Steps to Reproduce

  1. Subscribe to "pre_system" event and print some debug data
  2. Run a command with spark
  3. See that the debugging data is not printed

Expected Output

I expect spark to fire pre_system and post_system events.
After some research I found this commit, 775fa7d, which moved spark commands outside the CodeIgniter::run method.

Anything else?

I've searched the documentation and changelogs for info about this change. But wittout luck.
Will look for a workaround for now.

@Martin-4Spaces Martin-4Spaces added the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 21, 2024
@kenjis kenjis removed the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 21, 2024
@kenjis
Copy link
Member

kenjis commented Jan 21, 2024

Sorry, but this is not a bug.
Because these event points are for controllers, not for spark commands.
It was a bug that Spark triggered pre_system and post_system events.

https://codeigniter4.github.io/CodeIgniter4/extending/events.html#event-points

  • pre_system Called early during system execution. The URI, Request, and Response have been instantiated, but page cache checking, routing, and execution of “before” controller filters have not yet occurred.
  • post_system Called right before the final rendered page is sent to the browser, at the end of system execution, after the execution of “after” controller filters.

@kenjis
Copy link
Member

kenjis commented Jan 21, 2024

By the way, what do you want to do with these events when you run spark commands?

Probably you can fire these events if you extends CodeIgniter\CLI\Console::run() or CodeIgniter\CLI\Commands::run().

@kenjis kenjis added the wontfix Current code behavior being reported or fixed is intentional and won't be changed label Jan 21, 2024
@Martin-4Spaces
Copy link
Author

Use cases: External libraries that extend code igniter use pre_system to setup.
E.g

  • Extending routing to add API management capabilities. I use commands to interact with this extension.
  • Extending exception handler

pre_system and post_system have been a reliable way for extensions to initialize and exit.

Since commands have access to the CodeIgniter system, such as cache, config, cookies, email, files, etc. I would argue these events should be fired when commands are running.

@kenjis
Copy link
Member

kenjis commented Jan 22, 2024

I don't understand why a CLI command has something to do with routing.
Can I have a look at such a external library?

Spark is not for web applications, but for console applications.
And we separated them completely in v4.3.0. See #6110

@Martin-4Spaces
Copy link
Author

Routes can be stored in a database for API management purposes. Migrations/Seeds is used to save new routes. Spark is used to run migrations/seeds.
Here is an example of such a library https://github.com/4spacesdk/CI4RESTExtension/blob/master/Hooks.php
This is of cause just one example. Other libraries could benefit from these events. It is however always possible to find other solutions.

I have implemented a workaround that suites my CI4.4 project. And I will have to look for a replacement for pre_system and post_system in my libraries. Since I can't seem to convince you of the value of events like these. :)

@kenjis
Copy link
Member

kenjis commented Jan 22, 2024

Perhaps it is possible to add event points for Spark commands.

Let's wait and see what others have to say.

@lonnieezell
Copy link
Member

I think adding a couple of spark-specific events would work well. I can't imagine most needs would be the same between CLI and web but this would provide a solution for those that need it.

@kenjis
Copy link
Member

kenjis commented Jan 23, 2024

How about adding pre_command and post_command?

and If anyone is going to implement this, please send a PR to branch 4.5.
https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

@kenjis kenjis added the enhancement PRs that improve existing functionalities label Jan 24, 2024
@kenjis kenjis changed the title Bug: Spark no longer trigger pre_system and post_system events Spark no longer trigger pre_system and post_system events Jan 31, 2024
@kenjis
Copy link
Member

kenjis commented Feb 1, 2024

@Martin-4Spaces Check #8496

@kenjis kenjis added 4.5 and removed wontfix Current code behavior being reported or fixed is intentional and won't be changed labels Feb 1, 2024
@Martin-4Spaces
Copy link
Author

Awesome @kenjis ! Thank you.

@kenjis kenjis reopened this Feb 1, 2024
@kenjis
Copy link
Member

kenjis commented Feb 1, 2024

@Martin-4Spaces The PR is not merged yet.
If nobody approves it, it won't be merged.

If you believe the PR should be merged, approve it.
See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/approving-a-pull-request-with-required-reviews

@kenjis
Copy link
Member

kenjis commented Feb 4, 2024

Closed by #8496

@kenjis kenjis closed this as completed Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement PRs that improve existing functionalities
Projects
None yet
Development

No branches or pull requests

3 participants