Skip to content

Commit

Permalink
Merge pull request #1966 from sjinks/issue-1938
Browse files Browse the repository at this point in the history
Fix issues with the router
  • Loading branch information
Phalcon committed Feb 7, 2014
2 parents 8a1a504 + eed33fd commit 07809d4
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 213 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,12 @@
- Phalcon\Mvc\Model\Metadata now implements Phalcon\Mvc\Model\MetadataInterface (#1852)
- Phalcon\Mvc\Model\Valdator now implements Phalcon\Mvc\Model\ValidatorInterface (#1852)
- Phalcon\Mvc\View\Engine now implements Phalcon\Mvc\View\EngineInterface (#1852)
- Added support for default parameters in Router/URL generation (#1078)
- Phalcon\Mvc\Model\Validator::getOption()/getOptions()/isSetOption() methods are now public (#1904)
- Phalcon\Mvc\Model::findFirst() now works when phqlLiterals is false (#886)
- Fixed notices when phalcon.orm.column_renaming is 0 (#1801)
- Implemented reset() for metadata adapters other than memory (#1934)
- Relaxed the requirement for the route to start with a slash (#1938)
- Phalcon\Mvc\Router\Group: merge slashes when the route prefix ends and the route pattern starts with a slash (#1938)
- Phalcon\Paginator:
- Phalcon\Paginator\Adapter\Model returns correct results even when page number is incorrect (#1654)
- Optimized Phalcon\Paginator\Adapter\QueryBuilder (#1632)
Expand Down
143 changes: 30 additions & 113 deletions ext/kernel/framework/router.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,46 +101,10 @@ zval *phalcon_replace_marker(int named, zval *paths, zval *replacements, unsigne
return NULL;
}

static void phalcon_append_params_to_url(zval *params, smart_str *route_str)
{
HashPosition pos;
HashTable *ht;
zval **v, v_copy;
zval *v_copy_ptr = &v_copy;

assert(Z_TYPE_P(params) == IS_ARRAY);
ht = Z_ARRVAL_P(params);

for (
zend_hash_internal_pointer_reset_ex(ht, &pos);
zend_hash_get_current_data_ex(ht, (void**)&v, &pos) == SUCCESS;
) {
int use_copy = 0;

if (Z_TYPE_PP(v) != IS_STRING) {
zend_make_printable_zval(*v, &v_copy, &use_copy);
if (use_copy) {
v = &v_copy_ptr;
}
}

smart_str_appendl(route_str, Z_STRVAL_PP(v), Z_STRLEN_PP(v));

if (use_copy) {
zval_dtor(&v_copy);
}

zend_hash_move_forward_ex(ht, &pos);
if (pos) {
smart_str_appendc(route_str, '/');
}
}
}

/**
* Replaces placeholders and named variables with their corresponding values in an array
*/
void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval *replacements, zval *defaults TSRMLS_DC){
void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval *replacements TSRMLS_DC){

char *cursor, *marker = NULL;
unsigned int bracket_count = 0, parentheses_count = 0, intermediate = 0;
Expand All @@ -150,8 +114,6 @@ void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval
int i;
zval *replace, replace_copy;
int use_copy, looking_placeholder = 0;
int cut_url_at = 0;
int defaults_used = 0;

if (Z_TYPE_P(pattern) != IS_STRING || Z_TYPE_P(replacements) != IS_ARRAY || Z_TYPE_P(paths) != IS_ARRAY) {
ZVAL_NULL(return_value);
Expand All @@ -164,19 +126,21 @@ void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval
return;
}

cursor = Z_STRVAL_P(pattern);
if (*cursor == '/') {
++cursor;
i = 1;
}
else {
i = 0;
}

if (!zend_hash_num_elements(Z_ARRVAL_P(paths))) {
ZVAL_STRINGL(return_value, Z_STRVAL_P(pattern), Z_STRLEN_P(pattern), 1);
ZVAL_STRINGL(return_value, Z_STRVAL_P(pattern)+i, Z_STRLEN_P(pattern)-i, 1);
return;
}

cursor = Z_STRVAL_P(pattern);

/**
* Ignoring the first character, it must be a /
*/
cursor++;

for (i = 1; i < Z_STRLEN_P(pattern); i++) {
for (; i < Z_STRLEN_P(pattern); ++i) {

ch = *cursor;
if (ch == '\0') {
Expand All @@ -196,31 +160,17 @@ void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval
if (intermediate > 0) {
if (bracket_count == 0) {
replace = phalcon_replace_marker(1, paths, replacements, &position, cursor, marker);
if (!replace && (defaults != NULL)) {
position--;
replace = phalcon_replace_marker(1, paths, defaults, &position, cursor, marker);
defaults_used = 1;
} else {
defaults_used = 0;
}
if (replace) {
use_copy = 0;
if (Z_TYPE_P(replace) == IS_ARRAY) {
phalcon_append_params_to_url(replace, &route_str);
} else {
if (Z_TYPE_P(replace) != IS_STRING) {
zend_make_printable_zval(replace, &replace_copy, &use_copy);
if (use_copy) {
replace = &replace_copy;
}
}
smart_str_appendl(&route_str, Z_STRVAL_P(replace), Z_STRLEN_P(replace));
if (Z_TYPE_P(replace) != IS_STRING) {
zend_make_printable_zval(replace, &replace_copy, &use_copy);
if (use_copy) {
zval_dtor(&replace_copy);
replace = &replace_copy;
}
}
if (!defaults_used) {
cut_url_at = route_str.len;
smart_str_appendl(&route_str, Z_STRVAL_P(replace), Z_STRLEN_P(replace));
if (use_copy) {
zval_dtor(&replace_copy);
}
}
cursor++;
Expand All @@ -244,31 +194,17 @@ void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval
if (intermediate > 0) {
if (parentheses_count == 0) {
replace = phalcon_replace_marker(0, paths, replacements, &position, cursor, marker);
if (!replace && (defaults != NULL)) {
position--;
replace = phalcon_replace_marker(1, paths, defaults, &position, cursor, marker);
defaults_used = 1;
} else {
defaults_used = 0;
}
if (replace) {
use_copy = 0;
if (Z_TYPE_P(replace) == IS_ARRAY) {
phalcon_append_params_to_url(replace, &route_str);
} else {
if (Z_TYPE_P(replace) != IS_STRING) {
zend_make_printable_zval(replace, &replace_copy, &use_copy);
if (use_copy) {
replace = &replace_copy;
}
}
smart_str_appendl(&route_str, Z_STRVAL_P(replace), Z_STRLEN_P(replace));
if (Z_TYPE_P(replace) != IS_STRING) {
zend_make_printable_zval(replace, &replace_copy, &use_copy);
if (use_copy) {
zval_dtor(&replace_copy);
replace = &replace_copy;
}
}
if (!defaults_used) {
cut_url_at = route_str.len;
smart_str_appendl(&route_str, Z_STRVAL_P(replace), Z_STRLEN_P(replace));
if (use_copy) {
zval_dtor(&replace_copy);
}
}
cursor++;
Expand All @@ -284,38 +220,20 @@ void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval
if (intermediate > 0) {
if (ch < 'a' || ch > 'z' || i == (Z_STRLEN_P(pattern) - 1)) {
replace = phalcon_replace_marker(0, paths, replacements, &position, cursor, marker);
if (!replace && (defaults != NULL)) {
position--;
replace = phalcon_replace_marker(1, paths, defaults, &position, cursor, marker);
defaults_used = 1;
} else {
defaults_used = 0;
}
if (replace) {
use_copy = 0;
if (Z_TYPE_P(replace) == IS_ARRAY) {
phalcon_append_params_to_url(replace, &route_str);
} else {
if (Z_TYPE_P(replace) != IS_STRING) {
zend_make_printable_zval(replace, &replace_copy, &use_copy);
if (use_copy) {
replace = &replace_copy;
}
}
smart_str_appendl(&route_str, Z_STRVAL_P(replace), Z_STRLEN_P(replace));
if (Z_TYPE_P(replace) != IS_STRING) {
zend_make_printable_zval(replace, &replace_copy, &use_copy);
if (use_copy) {
zval_dtor(&replace_copy);
replace = &replace_copy;
}
}
if (!defaults_used) {
cut_url_at = route_str.len;
smart_str_appendl(&route_str, Z_STRVAL_P(replace), Z_STRLEN_P(replace));
if (use_copy) {
zval_dtor(&replace_copy);
}
}
looking_placeholder = 0;
if (i < (Z_STRLEN_P(pattern) - 1)) {
--i;
}

continue;
}
}
Expand All @@ -336,7 +254,6 @@ void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval

cursor++;
}
route_str.len = cut_url_at;
smart_str_0(&route_str);

if (route_str.len) {
Expand Down
2 changes: 1 addition & 1 deletion ext/kernel/framework/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@

/* Extract named parameters */
void phalcon_extract_named_params(zval *return_value, zval *str, zval *matches);
void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval *uri, zval *defaults TSRMLS_DC);
void phalcon_replace_paths(zval *return_value, zval *pattern, zval *paths, zval *uri TSRMLS_DC);

#endif /* PHALCON_KERNEL_FRAMEWORK_ROUTER_H */
34 changes: 23 additions & 11 deletions ext/mvc/router/group.c
Original file line number Diff line number Diff line change
Expand Up @@ -357,57 +357,69 @@ PHP_METHOD(Phalcon_Mvc_Router_Group, getRoutes){
/**
* Adds a route applying the common attributes
*
* @param string $patten
* @param string $pattern
* @param array $paths
* @param array $httpMethods
* @return Phalcon\Mvc\Router\Route
*/
PHP_METHOD(Phalcon_Mvc_Router_Group, _addRoute){

zval *pattern, *paths = NULL, *http_methods = NULL, *prefix, *prefix_pattern;
zval **pattern, **paths = NULL, **http_methods = NULL, *prefix, *prefix_pattern;
zval *default_paths, *merged_paths = NULL;

phalcon_fetch_params_ex(1, 2, &pattern, &paths, &http_methods);
PHALCON_ENSURE_IS_STRING(pattern);

PHALCON_MM_GROW();

phalcon_fetch_params(1, 1, 2, &pattern, &paths, &http_methods);

if (!paths) {
paths = PHALCON_GLOBAL(z_null);
paths = &PHALCON_GLOBAL(z_null);
}

if (!http_methods) {
http_methods = PHALCON_GLOBAL(z_null);
http_methods = &PHALCON_GLOBAL(z_null);
}

PHALCON_OBS_VAR(prefix);
phalcon_read_property_this(&prefix, this_ptr, SL("_prefix"), PH_NOISY_CC);

if (Z_TYPE_P(prefix) != IS_STRING) {
convert_to_string_ex(&prefix);
}

/**
* Add the prefix to the pattern
*/
PHALCON_INIT_VAR(prefix_pattern);
PHALCON_CONCAT_VV(prefix_pattern, prefix, pattern);

if (*(Z_STRVAL_PP(pattern)) == '/' && Z_STRVAL_P(prefix)[Z_STRLEN_P(prefix)-1] == '/') {
ZVAL_STRINGL(prefix_pattern, Z_STRVAL_P(prefix), Z_STRLEN_P(prefix)-1, 1);
concat_function(prefix_pattern, prefix_pattern, *pattern TSRMLS_CC);
}
else {
PHALCON_CONCAT_VV(prefix_pattern, prefix, *pattern);
}

default_paths = phalcon_fetch_nproperty_this(this_ptr, SL("_paths"), PH_NOISY_CC);

/**
* Check if the paths need to be merged with current paths
*/
if (Z_TYPE_P(default_paths) == IS_ARRAY && Z_TYPE_P(paths) == IS_ARRAY) {
if (Z_TYPE_P(default_paths) == IS_ARRAY && Z_TYPE_PP(paths) == IS_ARRAY) {
/**
* Merge the paths with the default paths
*/
PHALCON_INIT_VAR(merged_paths);
phalcon_fast_array_merge(merged_paths, &default_paths, &paths TSRMLS_CC);
phalcon_fast_array_merge(merged_paths, &default_paths, paths TSRMLS_CC);
} else {
merged_paths = paths;
merged_paths = *paths;
}

/**
* Every route is internally stored as a Phalcon\Mvc\Router\Route
*/
object_init_ex(return_value, phalcon_mvc_router_route_ce);
phalcon_call_method_p3_noret(return_value, "__construct", prefix_pattern, merged_paths, http_methods);
phalcon_call_method_p3_noret(return_value, "__construct", prefix_pattern, merged_paths, *http_methods);
phalcon_call_method_p1_noret(return_value, "setgroup", this_ptr);

phalcon_update_property_array_append(this_ptr, SL("_routes"), return_value TSRMLS_CC);
Expand Down
45 changes: 7 additions & 38 deletions ext/mvc/url.c
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,6 @@ PHP_METHOD(Phalcon_Mvc_Url, get){
zval *matched, *regexp;
int local = 1;

zval *namespace_name, *module_name;
zval *controller_name, *action_name;

zval *defaults;

phalcon_fetch_params_ex(0, 2, &uri, &args);

PHALCON_MM_GROW();
Expand All @@ -321,6 +316,7 @@ PHP_METHOD(Phalcon_Mvc_Url, get){

PHALCON_INIT_VAR(base_uri);
phalcon_call_method(base_uri, this_ptr, "getbaseuri");

if (Z_TYPE_PP(uri) == IS_ARRAY) {
if (!phalcon_array_isset_string_fetch(&route_name, *uri, SS("for"))) {
PHALCON_THROW_EXCEPTION_STR(phalcon_mvc_url_exception_ce, "It's necessary to define the route name with the parameter \"for\"");
Expand Down Expand Up @@ -350,40 +346,12 @@ PHP_METHOD(Phalcon_Mvc_Url, get){
PHALCON_VERIFY_INTERFACE(router, phalcon_mvc_routerinterface_ce);
phalcon_update_property_this(this_ptr, SL("_router"), router TSRMLS_CC);
}

PHALCON_INIT_VAR(defaults);
array_init_size(defaults, 4);

if (!phalcon_array_isset_string_fetch(&namespace_name, *uri, SS("namespace"))) {
namespace_name = phalcon_fetch_nproperty_this(router, SL("_defaultNamespace"), PH_NOISY_CC);
if (Z_TYPE_P(namespace_name) != IS_NULL) {
phalcon_array_update_string(&defaults, SL("namespace"), namespace_name, PH_COPY);
}
}

if (!phalcon_array_isset_string_fetch(&module_name, *uri, SS("module"))) {
module_name = phalcon_fetch_nproperty_this(router, SL("_defaultModule"), PH_NOISY_CC);
if (Z_TYPE_P(module_name) != IS_NULL) {
phalcon_array_update_string(&defaults, SL("module"), module_name, PH_COPY);
}
}

if (!phalcon_array_isset_string_fetch(&controller_name, *uri, SS("controller"))) {
controller_name = phalcon_fetch_nproperty_this(router, SL("_defaultController"), PH_NOISY_CC);
if (Z_TYPE_P(controller_name) != IS_NULL) {
phalcon_array_update_string(&defaults, SL("controller"), controller_name, PH_COPY);
}
}

if (!phalcon_array_isset_string_fetch(&action_name, *uri, SS("action"))) {
action_name = phalcon_fetch_nproperty_this(router, SL("_defaultAction"), PH_NOISY_CC);
if (Z_TYPE_P(action_name) != IS_NULL) {
phalcon_array_update_string(&defaults, SL("action"), action_name, PH_COPY);
}
}

PHALCON_OBS_VAR(route_name);
phalcon_array_fetch_string(&route_name, *uri, SL("for"), PH_NOISY);

/**
* Every route is uniquely differenced by a name
* Every route is uniquely identified by a name
*/
PHALCON_INIT_VAR(route);
phalcon_call_method_p1(route, router, "getroutebyname", route_name);
Expand All @@ -407,7 +375,8 @@ PHP_METHOD(Phalcon_Mvc_Url, get){
* Replace the patterns by its variables
*/
PHALCON_INIT_VAR(processed_uri);
phalcon_replace_paths(processed_uri, pattern, paths, *uri, defaults TSRMLS_CC);
phalcon_replace_paths(processed_uri, pattern, paths, *uri TSRMLS_CC);

PHALCON_CONCAT_VV(return_value, base_uri, processed_uri);
}
else {
Expand Down
Loading

0 comments on commit 07809d4

Please sign in to comment.