Skip to content
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.

Sign up for GitHub

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

[NFR] Allow Numeric Keys in Phalcon\Config #730

Closed
wants to merge 9 commits into from
Closed

[NFR] Allow Numeric Keys in Phalcon\Config #730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
73c5c02
WIP
sjinks Jun 26, 2013
4c190f7
WIP
sjinks Jun 26, 2013
ac4ce01
Merge sjinks/config-696-a
sjinks Jun 27, 2013
02ba04f
Fix spacing to minimize diff
sjinks Jun 27, 2013
c2f732c
Test cases
sjinks Jun 27, 2013
22a9616
Proper implementation for Phalcon\Config::get()
sjinks Jun 27, 2013
0151ac4
Check another code path in __construct() and merge()
sjinks Jun 27, 2013
107b072
Possible fix for #732
sjinks Jun 27, 2013
ce35d6d
Added test case for #732
sjinks Jun 27, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 80 additions & 21 deletions ext/config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ PHALCON_INIT_CLASS(Phalcon_Config){
PHP_METHOD(Phalcon_Config, __construct){

zval *array_config = NULL, *value = NULL, *key = NULL, *config_value = NULL;
zval *tmp = NULL, **key_or_tmp;
HashTable *ah0;
HashPosition hp0;
zval **hd;
Expand All @@ -106,19 +107,28 @@ PHP_METHOD(Phalcon_Config, __construct){

PHALCON_GET_HKEY(key, ah0, hp0);
PHALCON_GET_HVALUE(value);

if (likely(Z_TYPE_P(key) == IS_STRING)) {
key_or_tmp = &key;
}
else {
PHALCON_INIT_NVAR(tmp);
phalcon_cast(tmp, key, IS_STRING);
key_or_tmp = &tmp;
}

if (Z_TYPE_P(value) == IS_ARRAY) {
if (!phalcon_array_isset_long(value, 0)) {
if (zend_hash_get_current_key_type(Z_ARRVAL_P(value)) != HASH_KEY_IS_LONG) {
PHALCON_INIT_NVAR(config_value);
object_init_ex(config_value, phalcon_config_ce);
phalcon_call_method_p1_noret(config_value, "__construct", value);

phalcon_update_property_zval_zval(this_ptr, key, config_value TSRMLS_CC);
phalcon_update_property_zval_zval(this_ptr, *key_or_tmp, config_value TSRMLS_CC);
} else {
phalcon_update_property_zval_zval(this_ptr, key, value TSRMLS_CC);
phalcon_update_property_zval_zval(this_ptr, *key_or_tmp, value TSRMLS_CC);
}
} else {
phalcon_update_property_zval_zval(this_ptr, key, value TSRMLS_CC);
phalcon_update_property_zval_zval(this_ptr, *key_or_tmp, value TSRMLS_CC);
}

zend_hash_move_forward_ex(ah0, &hp0);
Expand Down Expand Up @@ -146,14 +156,28 @@ PHP_METHOD(Phalcon_Config, __construct){
*/
PHP_METHOD(Phalcon_Config, offsetExists){

zval *index;
zval *index, *tmp, **index_or_tmp;

PHALCON_MM_GROW();

phalcon_fetch_params(0, 1, 0, &index);
index_or_tmp = &index;

if (unlikely(Z_TYPE_P(index) != IS_STRING)) {
PHALCON_INIT_VAR(tmp);
phalcon_cast(tmp, index, IS_STRING);
index_or_tmp = &tmp;
}

if (phalcon_isset_property_zval(this_ptr, index TSRMLS_CC)) {
RETURN_TRUE;
if (phalcon_isset_property_zval(this_ptr, *index_or_tmp TSRMLS_CC)) {
RETVAL_TRUE;
}
else {
RETVAL_FALSE;
}
RETURN_FALSE;

PHALCON_MM_RESTORE();
return;
}

/**
Expand All @@ -170,7 +194,7 @@ PHP_METHOD(Phalcon_Config, offsetExists){
*/
PHP_METHOD(Phalcon_Config, get){

zval *index, *default_value = NULL, *value;
zval *index, *default_value = NULL, *value, *tmp, **index_or_tmp;

PHALCON_MM_GROW();

Expand All @@ -180,10 +204,19 @@ PHP_METHOD(Phalcon_Config, get){
PHALCON_INIT_VAR(default_value);
}

if (phalcon_isset_property_zval(this_ptr, index TSRMLS_CC)) {
if (Z_TYPE_P(index) == IS_STRING) {
index_or_tmp = &index;
}
else {
PHALCON_INIT_VAR(tmp);
phalcon_cast(tmp, index, IS_STRING);
index_or_tmp = &tmp;
}

if (phalcon_isset_property_zval(this_ptr, *index_or_tmp TSRMLS_CC)) {

PHALCON_OBS_VAR(value);
phalcon_read_property_zval(&value, this_ptr, index, PH_NOISY_CC);
phalcon_read_property_zval(&value, this_ptr, *index_or_tmp, PH_NOISY_CC);
if (PHALCON_IS_NOT_EMPTY(value)) {
RETURN_CCTOR(value);
}
Expand All @@ -204,14 +237,22 @@ PHP_METHOD(Phalcon_Config, get){
*/
PHP_METHOD(Phalcon_Config, offsetGet){

zval *index, *value;
zval *index, *value, *tmp;

PHALCON_MM_GROW();

phalcon_fetch_params(1, 1, 0, &index);

PHALCON_OBS_VAR(value);
phalcon_read_property_zval(&value, this_ptr, index, PH_NOISY_CC);
if (likely(Z_TYPE_P(index) == IS_STRING)) {
phalcon_read_property_zval(&value, this_ptr, index, PH_NOISY_CC);
}
else {
PHALCON_INIT_VAR(tmp);
phalcon_cast(tmp, index, IS_STRING);
phalcon_read_property_zval(&value, this_ptr, tmp, PH_NOISY_CC);
}

RETURN_CCTOR(value);
}

Expand All @@ -227,15 +268,18 @@ PHP_METHOD(Phalcon_Config, offsetGet){
*/
PHP_METHOD(Phalcon_Config, offsetSet){

zval *index, *value, *array_value = NULL;
zval *index, *value, *tmp, *array_value = NULL;
zval **index_or_tmp;

PHALCON_MM_GROW();

phalcon_fetch_params(1, 2, 0, &index, &value);
index_or_tmp = &index;

if (Z_TYPE_P(index) != IS_STRING) {
PHALCON_THROW_EXCEPTION_STR(phalcon_config_exception_ce, "Index key must be string");
return;
if (unlikely(Z_TYPE_P(index) != IS_STRING)) {
PHALCON_INIT_VAR(tmp);
phalcon_cast(tmp, index, IS_STRING);
index_or_tmp = &tmp;
}
if (Z_TYPE_P(value) == IS_ARRAY) {
PHALCON_INIT_VAR(array_value);
Expand All @@ -246,7 +290,7 @@ PHP_METHOD(Phalcon_Config, offsetSet){
PHALCON_CPY_WRT(array_value, value);
}

phalcon_update_property_zval_zval(this_ptr, index, array_value TSRMLS_CC);
phalcon_update_property_zval_zval(this_ptr, *index_or_tmp, array_value TSRMLS_CC);

PHALCON_MM_RESTORE();
}
Expand All @@ -265,6 +309,11 @@ PHP_METHOD(Phalcon_Config, offsetUnset){
zval *index;

phalcon_fetch_params(0, 1, 0, &index);
#if PHP_VERSION_ID < 50400
Z_OBJ_HANDLER_P(getThis(), unset_property)(getThis(), index TSRMLS_CC);
#else
Z_OBJ_HANDLER_P(getThis(), unset_property)(getThis(), index, 0 TSRMLS_CC);
#endif

RETURN_TRUE;
}
Expand All @@ -282,6 +331,7 @@ PHP_METHOD(Phalcon_Config, offsetUnset){
PHP_METHOD(Phalcon_Config, merge){

zval *config, *array_config, *value = NULL, *key = NULL, *active_value = NULL;
zval *tmp = NULL, **key_or_tmp;
HashTable *ah0;
HashPosition hp0;
zval **hd;
Expand All @@ -305,11 +355,20 @@ PHP_METHOD(Phalcon_Config, merge){
PHALCON_GET_HKEY(key, ah0, hp0);
PHALCON_GET_HVALUE(value);

if (Z_TYPE_P(key) == IS_STRING) {
key_or_tmp = &key;
}
else {
PHALCON_INIT_NVAR(tmp);
phalcon_cast(tmp, key, IS_STRING);
key_or_tmp = &tmp;
}

if (Z_TYPE_P(value) == IS_OBJECT) {
if (phalcon_isset_property_zval(this_ptr, key TSRMLS_CC)) {
if (phalcon_isset_property_zval(this_ptr, *key_or_tmp TSRMLS_CC)) {

PHALCON_OBS_NVAR(active_value);
phalcon_read_property_zval(&active_value, this_ptr, key, PH_NOISY_CC);
phalcon_read_property_zval(&active_value, this_ptr, *key_or_tmp, PH_NOISY_CC);
if (Z_TYPE_P(active_value) == IS_OBJECT) {
if (phalcon_method_exists_ex(active_value, SS("merge") TSRMLS_CC) == SUCCESS) {
phalcon_call_method_p1_noret(active_value, "merge", value);
Expand All @@ -319,7 +378,7 @@ PHP_METHOD(Phalcon_Config, merge){
}
}
}
phalcon_update_property_zval_zval(this_ptr, key, value TSRMLS_CC);
phalcon_update_property_zval_zval(this_ptr, *key_or_tmp, value TSRMLS_CC);

zend_hash_move_forward_ex(ah0, &hp0);
}
Expand Down
84 changes: 84 additions & 0 deletions unit-tests/ConfigTest.php
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,88 @@ public function testConfigMerge()

}

public function testIssue696()
{
$x = new Phalcon\Config(
array(
'some_property' => array(
1 => 'a',
2 => 'b',
)
)
);

// __construct()
$this->assertTrue(is_array($x->some_property));

$x = new Phalcon\Config(
array(
'some_property' => array(
'x' => 'y',
1 => 'a',
2 => 'b',
)
)
);

// __construct(), different code path
$this->assertTrue(!is_array($x->some_property));
$this->assertTrue($x->some_property instanceof Phalcon\Config);

// offsetExists
$this->assertTrue(isset($x->some_property[1]));
$this->assertTrue(isset($x->some_property[2]));
$this->assertTrue(!isset($x->some_property[3]));

// offsetGet
$this->assertEquals($x->some_property[1], 'a');
$this->assertEquals($x->some_property[2], 'b');

// offsetSet
$x->some_property[2] = 'c';
$this->assertEquals($x->some_property[2], 'c');

// toArray()
$expected = array('some_property' => array('x' => 'y', 1 => 'a', 2 => 'c'));
$this->assertEquals($x->toArray(), $expected);

// get()
$this->assertEquals($x->some_property->get(2, 'xxx'), 'c');
$this->assertEquals($x->some_property->get(5, 'xxx'), 'xxx');

// merge()
$y = new Phalcon\Config(
array(
'some_property' => array(
'a' => 'b',
2 => 'e',
3 => 'f',
)
)
);

$x->merge($y);
$expected = array('some_property' => array('x' => 'y', 'a' => 'b', 1 => 'a', 2 => 'e', 3 => 'f'));
$this->assertEquals($x->toArray(), $expected);

// __construct(), merge() - check different code path
$x = new Phalcon\Config(array('a' => 0, 1 => array('b' => 0, 1 => 'b')));
$y = new Phalcon\Config(array('a' => 0, 1 => array('b' => 0, 2 => 'c')));
$x->merge($y);
$expected = array('a' => 0, 1 => array('b' => 0, 1 => 'b', 2 => 'c'));
$this->assertEquals($x->toArray(), $expected);
}

public function testIssue732()
{
$a = new Phalcon\Config(array('a' => 0));
$this->assertTrue(isset($a['a']));
unset($a['a']);
$this->assertTrue(!isset($a['a']));

$a = new Phalcon\Config(array('a' => 0, 1 => 1));
$this->assertTrue(isset($a[1]));
unset($a[1]);
$this->assertTrue(!isset($a[1]));
}
}