Single-write handler and action suffix issue fix :

[adam-rocska]

Issue Description

• Type: bug fix

We have a heavily event-driven solution built using PhalconPHP in-depth. Therefore, while the dispatch-loop is running, there are certain occasions when we naturally perform forwards, handle exceptions and so on.

We use multiple different controller action suffixes based on execution context (managed by undisclosed preconditions).

The problem we were facing was, that whenever one of our listeners were forwarding and modifying the suffix of the running dispatcher, the new suffix was not taken into consideration.

I have found in the source code, that the problem's root cause is, that the controller action suffix is read once, before the dispatch loop (the while cycle) kicks off. This can easily be fixed, and I do not expect unwanted side-effects based on the source code, common sense, and local testing.

Patched versions / release branches

Version	Patch applied
2.0.0	moved local value definition within while loop
2.0.x	changed action method value assignment to getter instead of local string concatenation.
2.1.x	changed action method value assignment to getter instead of local string concatenation.
3.0.x	changed action method value assignment to getter instead of local string concatenation.
3.1.x	changed action method value assignment to getter instead of local string concatenation.
3.2.x	changed action method value assignment to getter instead of local string concatenation.
4.0.x	changed action method value assignment to getter instead of local string concatenation.

Patch explanation

My aim with the changes was to do as little change as possible with little to no possibility for unexpected side-effects.

moved local value definition within while loop

On version 2.0.0 the move refactor seemed to be the least risky and smallest change as I could not find the expected getter for the Controller. The patch would have felt half-assed if I'd only fix the action part.

changed action method value assignment to getter instead of local string concatenation.

On all other versions I could see, that the controller suffix was always part of the loop. Therefore, I just had to change the action method definition from string concatenation to the getter. The getter is a better approach as it respects the object state.

From : let actionMethod = actionName . actionSuffix;
To : let actionMethod = this->getActiveMethod();

In raising this pull request, I confirm the following (please check boxes):

• I have read and understood the Contributing Guidelines?
• I have checked that another pull request for this purpose does not exist.
• I wrote some tests for this PR.

The given method has no proper test coverage, and to write up the proper mocking, preconditions, post-conditions and assertions to assure, that this trivial change works would consume too much of my free time. We are talking about a ~275ish line long method, with a cyclomatic complexity way over 12 (just by looking at it, not counting).

[sergeyklay]

The 2.x.x branch is not longer supported. I'm sorry. Closing in favor of #12988

[adam-rocska]

@sergeyklay We would really appreciate if we wouldn't have to carry on with our internal isolated forks instead of the "official" phalcon repo. We are planning to perform an update but it has a lower priority in the next quite a few months.
— The reason why I patched the old unsupported branches was to be a good boy-scout.

Could this be an exception, as the change size is small in its nature?

P.S. : Of-course I don't expect outdated support from any other phalconer.

[adam-rocska]

@sergeyklay And excuse me for my manners : I thank you very much for your time & effort!

[niden]

@adam-rocska Thank you for the contribution. Sadly as Serghei wrote this branch is unsupported. You can however do the patch in your own fork and offer that functionality to your installation(s).

3.x is a LTS support (until 2019) but 2.x was not, this is why we stopped accepting PRs in the 2.x branches.

I understand this will be inconvenient for you given the time and effort you put into this but we cannot support 3 versions at the same time (4 is work in progress).

The 2.x branch has not been updated since we went to 3.x so whatever you have in your local fork is what is in this repo pretty much.

I hope you find the time to upgrade to v3.

[adam-rocska]

@niden Thank you for the explanation. I totally understand you folks. This means we'll carry on with the fork.