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

XmlDriver incorrectly parses boolean option values #6129

Closed
kalimatas opened this issue Nov 16, 2016 · 8 comments
Closed

XmlDriver incorrectly parses boolean option values #6129

kalimatas opened this issue Nov 16, 2016 · 8 comments
Assignees
Labels
Milestone

Comments

@kalimatas
Copy link

I have a mapping for a column like that:

<field name="alpha2Code" column="country_code" type="string" length="2" column-definition="CHAR(2) NOT NULL">
    <options>
        <option name="fixed">false</option>
    </options>
</field>

Despite that I specified false as a value of fixed option, it will be true, because in XmlDriver::_parseOptions method the string false will be casted to true with (bool):

if ($option->count()) {
    $value = $this->_parseOptions($option->children());
} else {
    // here
    $value = (string) $option;
}

It probably should be:

$value = $this->evaluateBoolean((string) $option);
@Ocramius
Copy link
Member

@kalimatas can this be converted into a test case?

@kalimatas
Copy link
Author

I'm on it right now.

@Ocramius
Copy link
Member

Tests in #6130

@kalimatas
Copy link
Author

I would like also to fix it, though I'm not sure, if maintaining an array of option names, that are supposed to be boolean in the _parseOptions() method , is a nice solution.

@Ocramius
Copy link
Member

@kalimatas give it a shot - it can be a quick fix with a later refactoring, but if there's a possible BC break, we need to expose that as well.

kalimatas pushed a commit to kalimatas/doctrine2 that referenced this issue Nov 16, 2016
kalimatas pushed a commit to kalimatas/doctrine2 that referenced this issue Nov 16, 2016
@kalimatas
Copy link
Author

I created a generic test for all drivers.

kalimatas pushed a commit to kalimatas/doctrine2 that referenced this issue Nov 17, 2016
@Ocramius
Copy link
Member

Closing as per work done in #6130

Ocramius added a commit that referenced this issue Nov 17, 2016
#6129 Added unit test for boolean option values.
@Ocramius Ocramius self-assigned this Nov 17, 2016
@Ocramius Ocramius added this to the 2.6.0 milestone Nov 17, 2016
@Ocramius
Copy link
Member

Note: this is only merged into 2.6.0 due to risk of regressions in the mapping driver change.

yvoyer pushed a commit to yvoyer/doctrine2 that referenced this issue Nov 21, 2016
yvoyer pushed a commit to yvoyer/doctrine2 that referenced this issue Nov 21, 2016
yvoyer pushed a commit to yvoyer/doctrine2 that referenced this issue Nov 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants