-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Config] extracted the xml parsing from XmlUtils::loadFile into XmlUt… #23482
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
Conversation
…tring (javiereguiluz) This PR was squashed before being merged into the 2.7 branch (closes symfony#23461). Discussion ---------- Use rawurlencode() to transform the Cookie into a string | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#23255 | License | MIT | Doc PR | - Commits ------- 025dfff Use rawurlencode() to transform the Cookie into a string
…patched on AccountStatusException (chalasr) This PR was merged into the 2.7 branch. Discussion ---------- [Security] Fix authentication.failure event not dispatched on AccountStatusException | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#18807 | License | MIT | Doc PR | n/a Authentication fails if the user exists but its account is disabled/expired/locked, the failure event should be dispatched in this case, so that you can hook into as for any authentication exception. Commits ------- 64c2efd [Security] Fix authentication.failure event not dispatched on AccountStatusException
…ils::load, so the parsing can be used with xml strings, as well.
The failing test |
|
||
try { | ||
XmlUtils::parse(file_get_contents($fixtures.'valid.xml'), array($mock, 'validate')); | ||
$this->fail(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better solution is to use @expectedException
and @expectedExceptionMessage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just re-used the way it has been done in the other tests in this class. Normally I'd go with the annotations, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other tests are written this way, because they try to test multiple cases in the same test. This is a legacy from tests written in the early days of Symfony's usage of PHPUnit.
*/ | ||
class XmlUtils | ||
{ | ||
const XML_IS_NOT_VALID_MESSAGE = 'The XML is not valid.'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 for defining a public constant with this message. We don't do it anywhere in Symfony
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay
this is a new feature, so it cannot go in 2.7. It must go in 3.4 |
I wasn't really sure whether you're handling this as a new feature or not. I'll go with 3.4. |
I'm going to reopen this against the master branch... I messed some things up here, I guess. |
@Basster not again master by against 3.4 please :) |
@nicolas-grekas sure, thx. |
…File into XmlUtils::parse (Basster) This PR was squashed before being merged into the 3.4 branch (closes #23485). Discussion ---------- [Config] extracted the xml parsing from XmlUtils::loadFile into XmlUtils::parse | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT I needed the xml parsing for an XML string generated in memory, so I extracted the actual parsing from XmlUtils::loadFile into XmlUtils::load. Re-opened after I messed some things up in #23482 Commits ------- 7473981 [Config] extracted the xml parsing from XmlUtils::loadFile into XmlUtils::parse
I needed the xml parsing for an XML string generated in memory, so I extracted the actual parsing from XmlUtils::loadFile into XmlUtils::parse.