-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Dispatcher tries to camel case twice #12829
Comments
@sergeyklay : I spent a decent amount of time documenting the issue. Preferably this would not be automatically closed out without at least a response from the Phalcon team. |
@jerejones Could you solve it? |
@niden Could you please take a look |
@jesseforrest Thank you for the verbose message. I will try and have a look at this this shortly. |
This is the same issue I faced trying to camelize the action name, in my last PR. |
So the question is where do we concentrate. |
If you can handle this Alex that would be great. |
I noticed where Zephir's |
Regarding the |
To not complicate things, maybe a private method in dispatcher is enough for these specific cases, of handler name and active method name. |
That should be fine :) |
This has been addressed |
Expected and Actual Behavior
The following line in the Dispatcher can cause the controller name to be incorrectly camel cased two times, which can cause routes to not work:
https://github.com/phalcon/cphalcon/blob/master/phalcon/dispatcher.zep#L781
For single word controllers (e.g.
User
) this is not a problem, but for multiple word controllers (e.g.GiftCard
) it is.The problem arises when routes are defined in either of the following formats:
IMO, the real problem is that Phalcon Router class is trying to be too accommodating and allowing the controller to be passed in as
GiftCard
,giftCard
orgift_card
.To solve the problem, without a change being made to Phalcon, I could have done 1 of 2 things.
The first thing was to change the controller names to be snake_case. Example:
I decided against this approach because engineers wouldn't be able to simply search across the code base for all uses of
GiftCard
anymore. I also didn't like this approach because it enforced that controllers were passed into the router in snake_case, but action names were camelCase.The second option was to override the default Dispatcher behavior. This was the approach I took for now until this issue can be discussed and resolved:
I will create another story for documentation too, since I feel like the following page should showcase more examples with multiple word controller/action names:
https://docs.phalconphp.com/en/3.0.2/reference/routing.html
I think the documentation and code should be explicit and enforce a certain way. Ideally the way we pass in controller and action names would be similar.
Another side note, which is important for reproducing. This bug only appears if your source code is on a case-sensitive operating system (e.g. linux). It will not appear on mac, or if your code is mounted to a mac OS, because mac treats the following the same:
GiftCardController.php
, giftcardController.php,
giFtCaRdCoNtroLLeR.php`, etc.Details
php --ri phalcon
)php -v
)The text was updated successfully, but these errors were encountered: