[FileSystem] explain possible failure of symlink creation in windows#1
Merged
romainneutron merged 1 commit intoromainneutron:FilesystemExceptionsfrom Jun 19, 2012
Conversation
romainneutron
added a commit
that referenced
this pull request
Jun 19, 2012
[FileSystem] explain possible failure of symlink creation in windows
romainneutron
pushed a commit
that referenced
this pull request
Jul 16, 2012
Commits ------- a20fc68 Merge pull request #1 from SamsonIT/FilesystemExceptions 8eca661 [FileSystem] explains possible failure of symlink creation in windows b1f8744 Add Changelog BC Break note 24eb396 [Filesystem] Added few new behaviors: Discussion ---------- [Filesystem] Consistence and enhancements for Filesystem Bug fix: no Feature addition: yes Backwards compatibility break: **yes** Symfony2 tests pass: yes Fixes the following tickets: None License of the code: MIT This PR adds features and introduce a backward compatibility break. features : - whenever an action fails, a \RuntimeException is thrown - add access to the second and third arguments of ``touch`` function - add a recursive option for chmod - add a chown method - add a chgrp method The backward compatibility break happens in the mkdir method : Before this PR, a boolean is returned ; true if all directories were created, false otherwise. It now returns nothing. --------------------------------------------------------------------------- by travisbot at 2012-05-18T14:26:42Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1367000) (merged 83cdd622 into 1e15f21). --------------------------------------------------------------------------- by fabpot at 2012-05-20T02:40:28Z To be consistent, we should throw exception whenever some operation fails. --------------------------------------------------------------------------- by romainneutron at 2012-05-20T21:10:23Z I fix the consistency ; mkdir now throws an exception if a directory creation fails. This introduce a BC break, see PR message which has been updated with all features and BC break. Added chgrp and chown methods Add options for touch Add recursive option for chmod --------------------------------------------------------------------------- by travisbot at 2012-05-20T21:11:47Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1383619) (merged a4d1eeb8 into 1407f11). --------------------------------------------------------------------------- by travisbot at 2012-05-22T10:49:06Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399027) (merged 7e14b6bd into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-22T10:58:10Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399083) (merged 71852653 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-22T11:18:44Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399194) (merged 7645bad3 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-23T18:21:47Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414091) (merged b049d5b1 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-23T18:26:19Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414123) (merged 34903466 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-29T16:07:26Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467173) (merged b1d1eb2e into adf07f1). --------------------------------------------------------------------------- by travisbot at 2012-05-29T16:19:38Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467261) (merged 42015ffa into adf07f1). --------------------------------------------------------------------------- by romainneutron at 2012-06-01T14:30:45Z Any news about this PR ? --------------------------------------------------------------------------- by stloyd at 2012-06-08T09:57:39Z @romainneutron You need to [squash](http://www.silverwareconsulting.com/index.cfm/2010/6/6/Using-Git-Rebase-to-Squash-Commits) your commits, and add more proper message in squashed commit i.e.: > [Filesystem] Added few new behaviors: * whenever an action fails, a `RuntimeException` is thrown * add access to the second and third arguments of `touch()` function * add a recursive option for `chmod()` * add a `chown()` method * add a `chgrp()` method > BC break: `mkdir()` function throw exception in case of failture instead of returning Boolean value. --------------------------------------------------------------------------- by romainneutron at 2012-06-08T10:59:55Z @stloyd squash done ! --------------------------------------------------------------------------- by travisbot at 2012-06-08T11:26:20Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1565540) (merged 8f55ddb6 into f8e68e5). --------------------------------------------------------------------------- by travisbot at 2012-06-08T11:41:45Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1566247) (merged 880312b6 into f8e68e5). --------------------------------------------------------------------------- by romainneutron at 2012-06-09T11:42:24Z I've added documentation to the Filesystem component : symfony/symfony-docs#1439 --------------------------------------------------------------------------- by travisbot at 2012-06-09T16:47:20Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577754) (merged 5647ad41 into f8a09db). --------------------------------------------------------------------------- by stloyd at 2012-06-13T14:47:31Z @romainneutron You probably need to rebase your code as some changes were merge into master for `Filesystem`. --------------------------------------------------------------------------- by romainneutron at 2012-06-13T15:17:31Z @stloyd rebase OK ! by the way, do you have any idea when/if this PR will be merged ? --------------------------------------------------------------------------- by travisbot at 2012-06-13T15:20:44Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1611591) (merged c8b86c68 into c07e916). --------------------------------------------------------------------------- by fabpot at 2012-06-16T16:40:50Z You need to add a note about the BC breaks in the CHANGELOG file. --------------------------------------------------------------------------- by fabpot at 2012-06-16T16:43:20Z Also, instead of using `\RuntimeException`, I would create a custom exception like we have done in other components (an interface + a RuntimeException that implements the interface and extends \RuntimeException). The exception name can be something like `IOException`. --------------------------------------------------------------------------- by travisbot at 2012-06-18T10:11:20Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645757) (merged 925a8234 into 0b8b76b). --------------------------------------------------------------------------- by stloyd at 2012-06-18T10:14:52Z @fabpot Anything blocking merge of this PR ? (tests are failing because of issue in master, not releted to this PR) --------------------------------------------------------------------------- by romainneutron at 2012-06-18T10:29:20Z @fabpot @stloyd the latest push was just a rebase push for PR 4577 (symfony#4577) I'm currently fixing the Exception and changelog things, I'll push very soon --------------------------------------------------------------------------- by romainneutron at 2012-06-18T10:44:38Z @fabpot I've added the exception and the exception interface, add the changelog info --------------------------------------------------------------------------- by travisbot at 2012-06-18T10:53:34Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645981) (merged 634d6fb9 into 0b8b76b). --------------------------------------------------------------------------- by romainneutron at 2012-06-18T11:08:43Z As reported by @stloyd the PR is failing due to an issue in the master, I re-push and trig the PR build when this issue is solved --------------------------------------------------------------------------- by travisbot at 2012-06-18T11:16:58Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1646006) (merged 2f65945a into 0b8b76b).
romainneutron
pushed a commit
that referenced
this pull request
Jan 3, 2013
[FrameworkBundle] Added tests for trusted_proxies configuration.
romainneutron
pushed a commit
that referenced
this pull request
Jan 3, 2013
This PR was merged into the 2.0 branch. Commits ------- f0743b1 Merge pull request #1 from pylebecq/2.0 555e777 [FrameworkBundle] Added tests for trusted_proxies configuration. a0e2391 [FrameworkBundle] used the new method for trusted proxies Discussion ---------- [FrameworkBundle] used the new method for trusted proxies This makes the framework bundle using the new method from the request class. --------------------------------------------------------------------------- by fabpot at 2012-12-05T10:38:20Z As this is a sensitive issue, can you add some tests? Thanks. --------------------------------------------------------------------------- by bamarni at 2012-12-06T13:00:24Z Well I don't know why it fails on travis, I can't run the full test suite locally because of a segfault but ```phpunit src/Symfony/Bundle/``` marks all the tests as passing. --------------------------------------------------------------------------- by fabpot at 2012-12-06T13:08:11Z But it looks like the failing tests come from what you've changed. --------------------------------------------------------------------------- by bamarni at 2012-12-06T13:29:33Z Yes, I'm not saying it's not my fault but I can't reproduce this as locally it tells me they pass, I'll try to fix this this evening. --------------------------------------------------------------------------- by bamarni at 2012-12-06T17:49:28Z Apparently it fails only when running the whole testsuite, looking at other travis builds I can see this one on 2.0 : https://travis-ci.org/symfony/symfony/jobs/3495511 which fails in a similar way than here (https://travis-ci.org/symfony/symfony/jobs/3530928). Because of a place trying to access an undefined $_SERVER key : ```PHP Notice: Undefined index: SCRIPT_NAME ...``` but I can't find where, and the stack trace references some phpunit classes. I'd be happy if someone could give me some pointers in here as I don't have any clue about how to fix this.. --------------------------------------------------------------------------- by bamarni at 2012-12-06T18:00:57Z As a consulsion I'd say I can't run the whole testsuite locally (it fails even when I revert my commit), so there is no reliable way for me to fix this, if anyone is up for continuing this feel free. --------------------------------------------------------------------------- by fabpot at 2012-12-11T09:47:48Z @bamarni Can you just update this PR with the code change and no tests at all? I will then finish the PR. Thanks. --------------------------------------------------------------------------- by bamarni at 2012-12-11T16:58:17Z @fabpot: thanks for helping me out on this, hope you won't run into the same issue!
romainneutron
pushed a commit
that referenced
this pull request
Jan 3, 2013
This PR was merged into the 2.1 branch. Commits ------- 81967f6 [Form] Fixed failing tests for DateTimeToStringTransformer. Discussion ---------- [Form] Fixed failing tests for DateTimeToStringTransformer Bug fix: no Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: - Todo: - License of the code: MIT Documentation PR: - Tests were only failing at the end of the month. Since February was used in the test cases, date was being moved to the next month (February has less days than other months). If a day is not passed, \DateTime's constructor will set it to the first day of the month: ```php var_dump(new \DateTime('2010-02')); object(DateTime)#1 (3) { ["date"]=> string(19) "2010-02-01 00:00:00" ["timezone_type"]=> int(3) ["timezone"]=> string(13) "Europe/London" } ``` \DateTime is used in the test assertions. However, DateTimeToStringTransformer::reverseTransform() uses \DateTime::createFromFormat(), which sets a missing day to the current day: ```php var_dump(\DateTime::createFromFormat("Y-m", '2010-02')); object(DateTime)#1 (3) { ["date"]=> string(19) "2010-03-01 20:09:26" ["timezone_type"]=> int(3) ["timezone"]=> string(13) "Europe/London" } ``` I changed the date in the test case to avoid failures. If we need to be sure that month's not going to be changed, I'll update my PR.
romainneutron
pushed a commit
that referenced
this pull request
Jan 3, 2013
This PR was merged into the master branch. Commits ------- b550677 [Finder] Fix the BSD adapter 2401274 [Finder] Added bsd adapter (need tests). Discussion ---------- [Finder] Adds bsd adapter. OK on mac os x. --------------------------------------------------------------------------- by fabpot at 2012-10-31T08:22:05Z Here are the results for the Finder tests on my Mac: ``` ............................................................... 63 / 181 ( 34%) ......................find: -regextype: unknown primary or operator F..............find: -regextype: unknown primary or operator find: -regextype: unknown primary or operator .find: -regextype: unknown primary or operator find: -regextype: unknown primary or operator ......................... 126 / 181 ( 69%) ....................................................... Time: 1 second, Memory: 10.75Mb There was 1 failure: 1) Symfony\Component\Finder\Tests\FinderTest::testIgnoreDotFiles with data set #1 (Symfony\Component\Finder\Adapter\PhpAdapter) Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( - 0 => '/var/folders/h7/55h7wcsx4g1cl...r/.bar' - 1 => '/var/folders/h7/55h7wcsx4g1cl...r/.foo' - 2 => '/var/folders/h7/55h7wcsx4g1cl...o/.bar' - 3 => '/var/folders/h7/55h7wcsx4g1cl...r/.git' - 4 => '/var/folders/h7/55h7wcsx4g1cl...er/foo' - 5 => '/var/folders/h7/55h7wcsx4g1cl...oo bar' - 6 => '/var/folders/h7/55h7wcsx4g1cl...ar.tmp' - 7 => '/var/folders/h7/55h7wcsx4g1cl...st.php' - 8 => '/var/folders/h7/55h7wcsx4g1cl...est.py' - 9 => '/var/folders/h7/55h7wcsx4g1cl...r/toto' ) .../src/Symfony/Component/Finder/Tests/Iterator/IteratorTestCase.php:25 .../src/Symfony/Component/Finder/Tests/FinderTest.php:207 phpunit:46 ``` --------------------------------------------------------------------------- by jfsimon at 2012-10-31T08:46:22Z @fabpot thank you! It seems I need to experiment a little more... --------------------------------------------------------------------------- by jfsimon at 2012-11-01T14:38:31Z @fabpot BSD adapter is OK on mac os x.
romainneutron
pushed a commit
that referenced
this pull request
Apr 6, 2013
Fixed test to use Reflection
romainneutron
pushed a commit
that referenced
this pull request
Apr 6, 2013
This PR was merged into the 2.1 branch. Commits ------- 27cc0df Merge pull request #1 from merk/class-loader/idempotent 95af84c Fixed test to use Reflection bb08247 [ClassLoader] tweaked test 73bead7 [ClassLoader] made DebugClassLoader idempotent Discussion ---------- [ClassLoader] made DebugClassLoader idempotent The DebugClassLoader will wrap itself if `enable()` is called multiple time, such as when running functional tests. Please merge to 2.2 and master ASAP. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ --------------------------------------------------------------------------- by kriswallsmith at 2013-03-07T16:38:55Z ping @fabpot: this will speed up lots of functional tests :) --------------------------------------------------------------------------- by kriswallsmith at 2013-03-08T04:51:51Z @fabpot fixed by @merk
romainneutron
pushed a commit
that referenced
this pull request
Apr 6, 2013
This PR was merged into the master branch. Commits ------- 0d6be2e Added Portuguese (Portugal) translation to Security 87df0b7 Merge pull request #1 from symfony/master Discussion ---------- Add Portuguese from Portugal Security translation Translation file for portuguese from Portugal added in Security component translations
romainneutron
pushed a commit
that referenced
this pull request
Mar 12, 2014
This PR was submitted for the 2.4 branch but it was merged into the 2.5-dev branch instead (closes symfony#9857). Discussion ---------- Form Debugger JavaScript improvements | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | - | Deprecations? | - | Tests pass? | - | Fixed tickets | symfony#9123 | License | MIT | Doc PR | - Commits ------- 406756d Reverted Sfjs.toggle change 226ac7c Reverted new image 53f164f Fixed asset function c763d65 Merge pull request #1 from bschussek/issue9857 5afbaeb [WebProfilerBundle] Enlarged the clickable area of the toggle button in the form tree 58559d3 [WebProfilerBundle] Moved toggle icon behind the headlines in the form debugger a0030b8 [WebProfilerBundle] Changed toggle color back to blue and made headlines in the form debugger clickable 505c5be [WebProfilerBundle] Added "use strict" statements ebf13ed [WebProfilerBundle] Inverted toggler images and improved button coloring 07994d5 [WebProfilerBundle] Improved JavaScript of the form debugger 11bfda6 [WebProfilerBundle] Vertically centered the icons in the form tree 52b1620 Fixed CS f21dab2 Added error badge 111a404 Made sections collapsable 60b0764 Improved form tree 0e03189 Expand tree
romainneutron
pushed a commit
that referenced
this pull request
Nov 15, 2014
…webmozart) This PR was merged into the 2.3 branch. Discussion ---------- [Intl] Integrated ICU data into Intl component #1 | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#11447, symfony#10807 | License | MIT | Doc PR | - This PR is an alternative implementation to symfony#11884. It depends on ~~symfony#11906~~ and ~~symfony#11907~~ being merged first (~~these are included in the diff until after a merge+rebase~~ merged+rebased now). With this PR, the ICU component becomes obsolete. The ICU data is bundled with Intl in two different formats: JSON and the binary ICU resource bundle format (version 2) readable by PHP's `\ResourceBundle` class. For a performance comparison between the two, see my [benchmark](/webmozart/json-res-benchmark). ~~The data is contained in two zip files: json.zip (2.6MB) and rb-v2.zip (3.8MB). The handler~~ ```php \Symfony\Component\Intl\Composer\ScriptHandler::decompressData() ``` ~~needs to be added as Composer hook and decompresses the data after install/update.~~ The data is included as text/binary now. Git takes care of the compression. Before this PR can be merged, I would like to find out what the performance difference between the two formats is in real applications. For that, I need benchmarks from some real-life applications which use ICU data - e.g. in forms (language drop-downs, country selectors etc.) - for both the JSON and the binary data. You can force either format to be used by hard-coding the return value of `Intl::detectDataFormat()` to `Intl::JSON` and `Intl::RB_V2` respectively. I'll also try to create some more realistic benchmarks. If JSON is not significantly slower/takes up significantly more memory than the binary format, we can drop the binary format altogether. Commits ------- be819c1 [Intl] Integrated ICU data into Intl component
romainneutron
pushed a commit
that referenced
this pull request
Nov 15, 2014
This PR was merged into the 2.6-dev branch. Discussion ---------- [Debug] expose object_handle | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This is a small enhancement to the symfony debug C extension that allows fetching an object's handle. This is the `#number` as displayed by var_dump(): `class stdClass**#1** (0) {}` This is required for VarDumper to be able to expose objects' handles and thus have more precise dumps. It will allow inspecting "same" relationships between two *different* dumps. Commits ------- 5f6b676 [Debug] expose object_handle
romainneutron
pushed a commit
that referenced
this pull request
Mar 19, 2015
Remove duplicate 'require' for symfony/symfony PR
romainneutron
pushed a commit
that referenced
this pull request
Mar 19, 2015
…owlan) This PR was merged into the 2.3 branch. Discussion ---------- [Yaml] Improve YAML boolean escaping | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#13209 | License | MIT | Doc PR | None This PR ensures that PHP [values which would be interpreted as booleans][1] in older versions of the YAML spec are escaped with single quotes when dumped by the Dumper. For example, dumping this: ```php array( 'country_code' => 'no', 'speaks_norwegian' => 'y', 'heating' => 'on', ) ``` Will produce this YAML: ```yaml country_code: 'no' speaks_norwegian: 'y' heating: 'on' ``` [1]: http://yaml.org/type/bool.html Commits ------- 8fa056b Inline private 'is quoting required' methods in Escaper afe827a Merge pull request #2 from larowlan/patch-2 a0ec0fe Add comment as requested 1e0633e Merge pull request #1 from larowlan/patch-1 81a8090 Remove duplicate 'require' 3760e67 [Yaml] Improve YAML boolean escaping
romainneutron
pushed a commit
that referenced
this pull request
Sep 21, 2015
…alue (ogizanagi) This PR was merged into the 2.7 branch. Discussion ---------- [Console] Fix first choice was invalid when using value | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This PR solves the following issues encountered using question helper and choices questions: - First choice was not selectable by value. - ChoiceList with associative choices with mixed string and int keys has same issue with first choice. - Fix inconsistency by always returning values as strings. First point exemple:  Last two points are mainly edge cases. Indeed, if a QuestionChoice has something like : ```php array( '0' => 'No environment', '1' => 'My environment 1', 'env_2' => 'My environment 2', 3 => 'My environment 3', ); ``` as choices, you will not be able to select the first choice and get an `InvalidArgumentException`: ``` There were 2 errors: 1) Symfony\Component\Console\Tests\Helper\QuestionHelperTest::testChoiceFromChoicelistWithMixedKeys with data set #0 ('0', '0') InvalidArgumentException: Value "0" is invalid 2) Symfony\Component\Console\Tests\Helper\QuestionHelperTest::testChoiceFromChoicelistWithMixedKeys with data set #1 ('No environment', '0') InvalidArgumentException: Value "No environment" is invalid ``` Moreover, even if you were able to select by value (`No environment`), you'll get an integer instead of a string: ``` Failed asserting that '0' is identical to 0. ``` For more consistency, the solution is to always return a string. The issue does not exist in 2.6, as the `QuestionChoice::getDefaultValidator` handled things differently. Commits ------- 03e4ab6 [Console] Fix first choice was invalid when using value
romainneutron
pushed a commit
that referenced
this pull request
Jan 18, 2016
…eprecation on-demand (nicolas-grekas) This PR was merged into the 2.8 branch. Discussion ---------- [Bridge\PhpUnit] Display the stack trace of a deprecation on-demand | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This PR has been essential into making symfony#16708, and I think everyone working on a 3.0 migration will need this. Example: ``` $ SYMFONY_DEPRECATIONS_HELPER=/FormExtensionBootstrap3LayoutTest/ ./phpunit src/Symfony/Bridge/Twig/ PHPUnit 4.8.18 by Sebastian Bergmann and contributors. Testing src/Symfony/Bridge/Twig/ ................................................ Remaining deprecation triggered by Symfony\Bridge\Twig\Tests\Extension\FormExtensionBootstrap3LayoutTest::testSingleChoiceGrouped: The value "false" for the "choices_as_values" option is deprecated since version 2.8 and will not be supported anymore in 3.0. Set this option to "true" and flip the contents of the "choices" option instead. Stack trace: #0 src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php(286): trigger_error() #1 src/Symfony/Component/OptionsResolver/OptionsResolver.php(962): Symfony\Component\Form\Extension\Core\Type\ChoiceType->Symfony\Component\Form\Extension\Core\Type\{closure}() #2 src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php(277): Symfony\Component\OptionsResolver\OptionsResolver->offsetGet() symfony#3 src/Symfony/Component/OptionsResolver/OptionsResolver.php(962): Symfony\Component\Form\Extension\Core\Type\ChoiceType->Symfony\Component\Form\Extension\Core\Type\{closure}() symfony#4 src/Symfony/Component/OptionsResolver/OptionsResolver.php(791): Symfony\Component\OptionsResolver\OptionsResolver->offsetGet() symfony#5 src/Symfony/Component/Form/ResolvedFormType.php(156): Symfony\Component\OptionsResolver\OptionsResolver->resolve() symfony#6 src/Symfony/Component/Form/FormFactory.php(119): Symfony\Component\Form\ResolvedFormType->createBuilder() symfony#7 src/Symfony/Component/Form/FormFactory.php(48): Symfony\Component\Form\FormFactory->createNamedBuilder() symfony#8 src/Symfony/Component/Form/Tests/AbstractBootstrap3LayoutTest.php(540): Symfony\Component\Form\FormFactory->createNamed() symfony#9 [internal function]: Symfony\Component\Form\Tests\AbstractBootstrap3LayoutTest->testSingleChoiceGrouped() symfony#10 {main} KO src/Symfony/Bridge/Twig/ ``` Commits ------- 5a88fb6 [Bridge\PhpUnit] Display the stack trace of a deprecation on-demand
romainneutron
pushed a commit
that referenced
this pull request
Oct 28, 2016
Prevent BC in instantiateObject
romainneutron
pushed a commit
that referenced
this pull request
Oct 28, 2016
This PR was merged into the 3.2-dev branch.
Discussion
----------
[Serializer] Argument objects
| Q | A
| ------------- | ---
| Branch? | 3.1
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | TODO
| Fixed tickets | none
| License | MIT
| Doc PR | TODO
Assuming with have the two following entities:
```php
namespace AppBundle\Entity;
class Dummy
{
public function __construct(int $id, string $name, string $email, AnotherDummy $anotherDummy)
{
$this->id = $id;
$this->name = $name;
$this->email = $email;
$this->anotherDummy = $anotherDummy;
}
}
class AnotherDummy
{
public function __construct(int $id, string $uuid, bool $isEnabled)
{
$this->id = $id;
$this->uuid = $uuid;
$this->isEnabled = $isEnabled;
}
}
```
Doing the following will fail:
```php
$serializer->denormalize(
[
'id' => $i,
'name' => 'dummy',
'email' => 'du@ex.com',
'another_dummy' => [
'id' => 1000 + $i,
'uuid' => 'azerty',
'is_enabled' => true,
],
],
\AppBundle\Entity\Dummy::class
);
```
with a type error, because the 4th argument passed to `Dummy::__construct()` will be an array. The following patch checks if the type of the argument is an object, and if it is tries to denormalize that object as well.
I'm not sure if it's me missing something or this is a use case that has been omitted (willingly or not), but if it's a valuable patch I would be happy to work on finishing it.
Commits
-------
988eba1 fix tests
98bcb91 Merge pull request #1 from dunglas/theofidry-feature/param-object
7b5d55d Prevent BC in instantiateObject
e437e04 fix reflection type
3fe9802 revert CS
5556fa5 fix
d4cdb00 fix CS
93608dc Add deprecation message
f46a176 Apply patch
f361e52 fix tests
4884a2e f1
e64e999 Address comments
e99a90b Add tests
7bd4ac5 Test
romainneutron
pushed a commit
that referenced
this pull request
Feb 28, 2017
Update HttpBasicLdapFactory
romainneutron
pushed a commit
that referenced
this pull request
Feb 28, 2017
…capable of searching for the DN (lsmith77, nietonfir) This PR was merged into the 3.3-dev branch. Discussion ---------- [Security] make LdapBindAuthenticationProvider capable of searching for the DN | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#16823, symfony#20905 | License | MIT | Doc PR | symfony/symfony-docs#7420 I guess due to the separation between the user and auth provider something like the following isn't ok (note: the following works just fine for me but if course in the end the username is the DN and not the user provided shorter username): ```diff diff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php index 5ebb09a..18d7825 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php @@ -70,7 +70,7 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider */ protected function checkAuthentication(UserInterface $user, UsernamePasswordToken $token) { - $username = $token->getUsername(); + $username = $user->getUsername(); $password = $token->getCredentials(); if ('' === $password) { @@ -78,10 +78,7 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider } try { - $username = $this->ldap->escape($username, '', LdapInterface::ESCAPE_DN); - $dn = str_replace('{username}', $username, $this->dnString); - - $this->ldap->bind($dn, $password); + $this->ldap->bind($username, $password); } catch (ConnectionException $e) { throw new BadCredentialsException('The presented password is invalid.'); } diff --git a/src/Symfony/Component/Security/Core/User/LdapUserProvider.php b/src/Symfony/Component/Security/Core/User/LdapUserProvider.php index fc42419..8194c4c 100644 --- a/src/Symfony/Component/Security/Core/User/LdapUserProvider.php +++ b/src/Symfony/Component/Security/Core/User/LdapUserProvider.php @@ -115,7 +115,7 @@ class LdapUserProvider implements UserProviderInterface { $password = $this->getPassword($entry); - return new User($username, $password, $this->defaultRoles); + return new User($entry->getDn(), $password, $this->defaultRoles); } /** ``` Therefore I created an entire new auth provider. Commits ------- 8ddd533 Merge pull request #1 from nietonfir/http_basic_ldap a783e5c Update HttpBasicLdapFactory a30191f make LdapBindAuthenticationProvider capable of searching for the DN
romainneutron
pushed a commit
that referenced
this pull request
Feb 28, 2017
…utput trimming (ogizanagi) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] Revert AbstractDescriptorTest output trimming | Q | A | ------------- | --- | Branch? | master | Bug fix? | no, but fixes an annoying/unhelping test output and fixes false positives | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | https://github.com/symfony/symfony/pull/21129/files#diff-0e52187cbf1067a310538287da74ddb5R178 | License | MIT | Doc PR | N/A #### Before output when having a failing test, for instance, in a `TextDescriptor` fixture: ```diff There was 1 failure: 1) Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor\TextDescriptorTest::testDescribeContainerDefinitionWhichIsAnAlias with data set #1 (Symfony\Component\DependencyInjection\Alias Object (...), ' // This serv...------', Symfony\Component\DependencyInjection\ContainerBuilder Object (...), array('alias_2')) Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -' // This service is an alias for the service service_2 Information for Service "service_2" =================================== ------------------ --------------------------------- Option Value ------------------ --------------------------------- Class Full\Qualified\Class2 Tags tag1 (attr1: val1, attr2: val2) tag1 (attr3: val3) tag2 Calls setMailer Public no Synthetic yes Lazy no Shared yes Abstract no Autowired no Autowiring Types - Required File /path/to/file Factory Service factory.service Factory Method get ------------------ ---------------------------------' +' // This service is an alias for the service service_2 Information for Service "service_2" =================================== ------------------ --------------------------------- Option Value ------------------ --------------------------------- Service ID service_2 Class Full\Qualified\Class2 Tags tag1 (attr1: val1, attr2: val2) tag1 (attr3: val3) tag2 Calls setMailer Public no Synthetic yes Lazy no Shared yes Abstract no Autowired no Autowiring Types - Required File /path/to/file Factory Service factory.service Factory Method get ------------------ ---------------------------------' ``` #### After: ```diff There was 1 failure: 1) Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor\TextDescriptorTest::testDescribeContainerDefinitionWhichIsAnAlias with data set #1 (Symfony\Component\DependencyInjection\Alias Object (...), ' // This serv...------', Symfony\Component\DependencyInjection\ContainerBuilder Object (...), array('alias_2')) Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ ------------------ --------------------------------- + Service ID service_2 Class Full\Qualified\Class2 Tags tag1 (attr1: val1, attr2: val2) tag1 (attr3: val3) tag2 Calls setMailer Public no Synthetic yes Lazy no Shared yes Abstract no Autowired no Autowiring Types - Required File /path/to/file Factory Service factory.service Factory Method get ------------------ ---------------------------------' ``` Commits ------- efd00ba [FrameworkBundle] Revert AbstractDescriptorTest output trimming
romainneutron
pushed a commit
that referenced
this pull request
Feb 28, 2017
…er in autowired classes (brainexe) This PR was squashed before being merged into the 3.1 branch (closes symfony#21372). Discussion ---------- [DependencyInjection] Fixed variadic method parameter in autowired classes | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | -- | License | MIT Autowiring classes containing methods with variadic method parameter throws a ReflectionException in compile process: ``` PHP Fatal error: Uncaught ReflectionException: Internal error: Failed to retrieve the default value in /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php:437 Stack trace: #0 /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php(437): ReflectionParameter->getDefaultValue() #1 /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php(80): Symfony\Component\DependencyInjection\Compiler\AutowirePass::getResourceMetadataForMethod(Object(ReflectionMethod)) #2 /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php(105): Symfony\Component\DependencyInjection\Compiler\AutowirePass::createResourceForClass(Object(ReflectionClass)) symfony#3 /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php(48): Symfony\Component\DependencyInjection\Compiler\AutowirePass->completeDefinition('__controller.Sw...', Object(Symfony\Component\DependencyInjection\Definition), Array) symfony#4 /.../vendor/symfony/dependency-injection/Compiler/Compiler in /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php on line 437 ``` **Example:** ``` <?php class FooVariadic { public function bar(...$arguments) { } } $method = new ReflectionMethod(FooVariadic::class, 'bar'); $parameter = $method->getParameters()[0]; $parameter->getDefaultValue(); // -> ReflectionException: Internal error: Failed to retrieve the default value in ... ``` Commits ------- a7f63de [DependencyInjection] Fixed variadic method parameter in autowired classes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As requested in this comment symfony#4577 (comment), PR symfony#4577 can be a valuable addition to a failure in creating symlinks.