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.

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

Allow '' to be a valid key for Hash, and Session #9635

Merged
merged 2 commits into from Oct 27, 2016
Merged

Allow '' to be a valid key for Hash, and Session #9635

merged 2 commits into from Oct 27, 2016

Conversation

markstory
Copy link
Member

By removing a bunch of empty() guards we can make '' behave like all the other key names. This does change the existing behavior/tests around '' key, but I think that is ok given the need to manipulate ''.

Refs #9632

By removing a bunch of empty() guards we can make '' behave like all the
other key names. This does change the existing behavior/tests around ''
key, but I think that is ok given the need to manipulate ''.

Refs #9632
@markstory markstory added this to the 2.9.2 milestone Oct 21, 2016
@ADmad
Copy link
Member

ADmad commented Oct 21, 2016

In what cases does using empty string as key actually make sense?

In reference to #9632 i would rather prevent empty string being used when saving cache. That way deleting it won't be even an issue.

@dereuromark
Copy link
Member

dereuromark commented Oct 21, 2016

@ADmad How can you prevent that for deeply nested array being written to the session?
You cannot traverse the whole tree when saving

$this->Session->write('Key', $veryDeepArray);

But you still need to read it when traversing over an array of those keys and in case there is some empty string in it:

foreach ($array as $v) {
    ->read('Key.foo.bar. ' $v ' .baz')
}

or sth?
I admit it is an edge case, but if it is possible to fix up rather than breaking it half way in a different direction with exceptions, I think that is the better way.

@ADmad
Copy link
Member

ADmad commented Oct 21, 2016

Having an empty key is almost always an error but i see there's no easy way to prevent it. So i guesa the only option is to allow deleting such a key.

@markstory markstory merged commit 1a58d15 into 2.x Oct 27, 2016
@markstory markstory deleted the issue-9632 branch October 27, 2016 01:16
markstory added a commit that referenced this pull request Oct 27, 2016
ndm2 pushed a commit to ndm2/cakephp that referenced this pull request Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants