Cphalcon: Dispatcher tries to camel case twice

[jesseforrest]

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:

$router->add('/gift-card', ['controller' => 'GiftCard', 'action' => 'showGiftCard']);
$router->add('/gift-card', 'GiftCard::showGiftCard');

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 or gift_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:

$router->add('/gift-card', ['controller' => 'gift_card', 'action' => 'showGiftCard']);
$router->add('/gift-card', 'gift_card::showGiftCard');

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:

<?php
use Phalcon\Mvc\Dispatcher;

class FixedDispatcher extends Dispatcher
{
    public function getHandlerClass(): string
    {
        parent::getHandlerClass();
        return $this->_handlerName . $this->_handlerSuffix;
    }
}

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

Describe what you are trying to achieve and what goes wrong.

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

• Phalcon version: (php --ri phalcon)

Web framework delivered as a C-extension for PHP
phalcon => enabled
Author => Phalcon Team and contributors
Version => 3.0.3
Build Date => Dec 24 2016 19:16:23
Powered by Zephir => Version 0.9.5a-dev

Directive => Local Value => Master Value
phalcon.db.escape_identifiers => On => On
phalcon.db.force_casting => Off => Off
phalcon.orm.events => On => On
phalcon.orm.virtual_foreign_keys => On => On
phalcon.orm.column_renaming => On => On
phalcon.orm.not_null_validations => On => On
phalcon.orm.exception_on_failed_save => Off => Off
phalcon.orm.enable_literals => On => On
phalcon.orm.late_state_binding => Off => Off
phalcon.orm.enable_implicit_joins => On => On
phalcon.orm.cast_on_hydrate => Off => Off
phalcon.orm.ignore_unknown_columns => Off => Off

• PHP Version: (php -v)

PHP 7.0.17-2+deb.sury.org~trusty+1 (cli) (built: Mar 15 2017 09:38:47) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.0.17-2+deb.sury.org~trusty+1, Copyright (c) 1999-2017, by Zend Technologies

• Operating System:

Ubuntu

• Installation type: Compiling from source || installing via package manager
• Zephir version (if any):
• Server: Apache
• Other related info (Database, table schema):

[jesseforrest]

@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.

[takahashiyuya]

@jerejones Could you solve it?

[sergeyklay]

@niden Could you please take a look

[niden]

@jesseforrest Thank you for the verbose message. I will try and have a look at this this shortly.

[alexbusu]

This is the same issue I faced trying to camelize the action name, in my last PR.
@niden it seems the problems would be solved if the camelize function would not "strtolower" the input before apply the formatting. Currently MyHandler string for controller input in router is returned as MyhandlerController in dispatcher.

[niden]

So the question is where do we concentrate. camelize function offered by Zephir or fix this here? I fear that messing up with the camelize will break more things :(

[niden]

If you can handle this Alex that would be great.

[alexbusu]

I noticed where Zephir's camelize() applies this behaviour.
That part is not to be touched though, because it's used in several places across the framework, in sensitive places (adapters name camelize + prepend to NS, Models getters/setters and other stuff like this).
What about Phalcon's own version of camelize (Text::toCamelCase() or something)?
@niden Yes, I could do it.

[niden]

Regarding the toCamelize this needs much more discussion. I don;t like adding one method just for one area. We have to check all the _camelization_ (if that is even a word) in the framework and come up with a robust solution really.

[alexbusu]

To not complicate things, maybe a private method in dispatcher is enough for these specific cases, of handler name and active method name.

[niden]

That should be fine :)

[niden]

This has been addressed