[FrameworkBundle] add "mapping" configuration key at validation secti… #19086

davewwww · 2016-06-17T12:27:35Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#15655
License	MIT
Doc PR	symfony/symfony-docs#7407

This feature allows you, to define additional validation files or directories which are not in the 'Bundle*/Resources/config/' directory.

#config.yml
framework:
  validation:
    mapping:
      paths:
        - "path/to/file/validation.yml"
        - "path/to/file/validation.xml"
        - "path/to/another/directory"

GromNaN · 2016-06-17T12:37:26Z

Supporting files and directories in the same list would be a better developer experience:

#config.yml
framework:
  validation:
    mapping:
      - "path/to/file/validation.yml"
      - "path/to/file/validation.xml"
      - "path/to/another/directory"

linaori · 2016-06-17T13:25:08Z

Check out how the translation does it, would be nice if it's got a similar configuration implementation.

davewwww · 2016-06-17T14:28:14Z

if you guys prefer to combine files and dirs, then I can do that.

linaori · 2016-06-17T19:41:30Z

https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php#L454
https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L704

At the translator part it called path(s), but I think it only loads from a root dir. Why not let the validation behave the same?

davewwww · 2016-06-22T09:24:55Z

Is there anything I can do to finish this ticket?

GuilhemN · 2016-06-23T16:26:32Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+     */
+    private function getValidatorMappingFilesFromDir($dir, array &$files)
+    {
+        foreach (Finder::create()->files()->in($dir)->name('/\.xml|yml$/') as $file) {


.yaml should be supported I guess ?

Shouldn't it rather be '/\.(xml|yml)$/' ?
(so '/\.(xml|yml|yaml)$/' with the .yaml extension)

EDIT: Then \.(xml|ya?ml)$ according to the comment below.

You can use ya?ml too

GuilhemN · 2016-06-23T16:33:50Z

@iltar the translations are meant to be put in a folder as there are at least two versions of the same file but the validation files can be in the root of a config folder so imo we should support files and dirs.

@davewwww isn't it possible to merge the two config options under paths ?

davewwww · 2016-06-30T13:02:27Z

i've merged the two "files" and "dirs" options to a single "paths" option.

xabbuh · 2016-07-04T11:08:46Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

@@ -180,6 +180,13 @@
        <xsd:attribute name="cache" type="xsd:string" />
        <xsd:attribute name="enable-annotations" type="xsd:boolean" />
        <xsd:attribute name="static-method" type="xsd:boolean" />
+        <xsd:attribute name="mapping" type="validation_mapping" />


mapping is no attribute (it must be an element)

xabbuh · 2016-07-04T11:09:37Z

Status: Needs work

davewwww · 2016-07-06T08:36:00Z

@xabbuh can I do anything else?

xabbuh · 2016-07-06T20:45:31Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+                $this->getValidatorMappingFilesFromDir($path, $files);
+            } elseif (is_file($path)) {
+                if (preg_match('/\.(xml|ya?ml)$/', $path, $matches)) {
+                    $files[$matches[0]][] = realpath($path);


$matches[0] will contain the string yaml when the file extension is .yaml which would now be ignored.

you are right. i added a .yaml file extension support and added additional tests.

lunetics · 2016-11-19T00:55:42Z

This sounds like a good solution when not using bundles at all. Anything against merging that?

davewwww · 2016-11-28T12:02:59Z

@xabbuh: I would like to move on here, what can I do next?

fabpot · 2017-01-10T20:59:21Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+     * Search for validation files in directory.
+     *
+     * @param string $dir   The directory that contains the validation files
+     * @param array  $files A array reference of validation files


fabpot · 2017-01-10T20:59:33Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+     * Get all validation files.
+     *
+     * @param ContainerBuilder $container A ContainerBuilder instance
+     * @param array            $files     A array reference of validation files


fabpot · 2017-01-10T20:59:57Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+     * Get all validation files from config.
+     *
+     * @param array $config A validation configuration array
+     * @param array $files  A array reference of validation files


fabpot · 2017-01-10T21:00:14Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php


-        return $files;
+    /**
+     * Search for validation files in directory.


fabpot · 2017-01-10T21:00:20Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+    }
+
+    /**
+     * Get all validation files from config.


fabpot · 2017-01-10T21:00:38Z

Can you add a note about this new feature in the CHANGELOG?

HeahDude

👍

HeahDude · 2017-01-19T18:16:15Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -979,41 +982,76 @@ private function registerValidationConfiguration(array $config, ContainerBuilder
        }
    }

-    private function getValidatorMappingFiles(ContainerBuilder $container)
+    /**
+     * Get all validation files.


I think the whole phpdoc block can be removed. It does not add that much value (and is for a private method anyway).

Same for the other ones.

fabpot · 2017-01-20T15:42:57Z

Tests do not pass on Windows

davewwww · 2017-01-20T16:11:55Z

Yes, because
TestBundle\Resources\config\validation.xml
is not:
TestBundle/Resources/config/validation.xml

$this->assertStringEndsWith('TestBundle/Resources/config/validation.xml', $xmlMappings[1]);

appears the error because of my changes?

fabpot · 2017-01-20T16:15:00Z

@davewwww Apparently, yes

davewwww · 2017-01-20T16:28:25Z

The problem is solved when I remove the realpath()

if (is_file($file = $dirname.'/Resources/config/validation.xml')) {
   $files['xml'][] = realpath($file);
 }

fabpot · 2017-01-22T18:41:58Z

@davewwww This now looks good to me. But before I can merge, can you squash your commit or at least rebase on current master (to get rid of the merge commit which prevents me to merge this PR cleanly).

Also, can you submit a PR to update the Symfony docs? Between, can you also change the description of this PR to reflect the latest version of this PR?

Thanks

davewwww · 2017-01-23T14:56:43Z

spuashed, pr updated and doc pr submitted

fabpot · 2017-01-23T18:23:24Z

Thank you @davewwww.

…alidation secti… (davewwww) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] add "mapping" configuration key at validation secti… | Q | A | | --- | --- | | Bug fix? | no | | New feature? | yes | | BC breaks? | no | | Deprecations? | no | | Tests pass? | yes | | Fixed tickets | #15655 | | License | MIT | | Doc PR | symfony/symfony-docs#7407 | This feature allows you, to define additional validation files or directories which are not in the 'Bundle*/Resources/config/' directory. ``` yaml #config.yml framework: validation: mapping: paths: - "path/to/file/validation.yml" - "path/to/file/validation.xml" - "path/to/another/directory" ``` Commits ------- d696cfb [FrameworkBundle] Configurable paths for validation files 60d7d43 fix merge 61475b5 Merge branch '3.2' ba41e70 Merge branch '3.1' into 3.2 4268aba Merge branch '2.8' into 3.1 3faf655 Merge branch '2.7' into 2.8 e95fc09 fix getMock usage 482828c fix merge ed5eb6d bug #21372 [DependencyInjection] Fixed variadic method parameter in autowired classes (brainexe) a7f63de [DependencyInjection] Fixed variadic method parameter in autowired classes 9ef4271 minor #21371 [Validator] update German translation (xabbuh) f920e61 update German translation 41c72ab minor #21335 [Validator] Improved error message for missing upload_tmp_dir (Breuls) afbf227 [Validator] Improved error message for missing upload_tmp_dir

…es (davewwww) This PR was merged into the master branch. Discussion ---------- [FrameworkBundle] Configurable paths for validation files add doc for symfony/symfony#19086 Commits ------- 793e9e5 [FrameworkBundle] Configurable paths for validation files

stof · 2017-03-08T10:43:06Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+        foreach ($config['mapping']['paths'] as $path) {
+            if (is_dir($path)) {
+                $this->getValidatorMappingFilesFromDir($path, $files);
+            } elseif (is_file($path)) {


this must use the ContainerBuilder API to register resources to invalidate the cache when files are changing in debug mode

indeed, see #21927

…tion mapping (chalasr) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] Add missing resource tracking for validation mapping | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19086 (comment) | License | MIT | Doc PR | n/a https://github.com/symfony/symfony/pull/21927/files?w=1 Commits ------- 0a19cbe Add missing resource tracking for validation mapping

…tion mapping (chalasr) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] Add missing resource tracking for validation mapping | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony/symfony#19086 (comment) | License | MIT | Doc PR | n/a https://github.com/symfony/symfony/pull/21927/files?w=1 Commits ------- 0a19cbe Add missing resource tracking for validation mapping

…g paths (chalasr) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] Allow to configure Serializer mapping paths | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #21187 | License | MIT | Doc PR | todo Follows #19086 for the Serializer Commits ------- 5446903 [FrameworkBundle] Allow configuring serializer mapping paths

…g paths (chalasr) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] Allow to configure Serializer mapping paths | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #21187 | License | MIT | Doc PR | todo Follows symfony/symfony#19086 for the Serializer Commits ------- 5446903296 [FrameworkBundle] Allow configuring serializer mapping paths

carsonbot added Status: Needs Review FrameworkBundle Feature labels Jun 17, 2016

davewwww mentioned this pull request Jun 17, 2016

[FrameworkBundle] "mappings" for validation #13878

Closed

GuilhemN reviewed Jun 23, 2016
View reviewed changes

xabbuh reviewed Jul 4, 2016
View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Jul 4, 2016

xabbuh reviewed Jul 6, 2016
View reviewed changes

nicolas-grekas added this to the 3.x milestone Dec 6, 2016

ogizanagi mentioned this pull request Jan 6, 2017

[Serializer] Can't use config files without bundle #21187

Closed

fabpot reviewed Jan 10, 2017

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

}

/**

* Get all validation files from config.

Copy link

Member

fabpot Jan 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gets

carsonbot added the Status: Reviewed label Jan 15, 2017

HeahDude approved these changes Jan 19, 2017

View reviewed changes

davewwww force-pushed the master branch from dc93246 to cb9c18b Compare January 23, 2017 10:38

davewwww mentioned this pull request Jan 23, 2017

[FrameworkBundle] Configurable paths for validation files symfony/symfony-docs#7407

Merged

[FrameworkBundle] Configurable paths for validation files

d696cfb

davewwww force-pushed the master branch from cb9c18b to d696cfb Compare January 23, 2017 14:04

fabpot merged commit d696cfb into symfony:master Jan 23, 2017

This was referenced Mar 7, 2017

Missing documentation for the framework.validation.mapping config option symfony/symfony-docs#7593

Closed

[FrameworkBundle] Allow to configure Serializer mapping paths #21924

Merged

stof reviewed Mar 8, 2017

View reviewed changes

chalasr mentioned this pull request Mar 8, 2017

[FrameworkBundle] Add missing resource tracking for validation mapping #21927

Merged

nicolas-grekas modified the milestones: 3.x, 3.3 Mar 24, 2017

fabpot mentioned this pull request May 1, 2017

Release v3.3.0-BETA1 #22603

Merged

ogizanagi mentioned this pull request May 19, 2017

[FrameworkBundle] No way to specify validation config outside of the bundle #22772

Closed

[FrameworkBundle] add "mapping" configuration key at validation secti… #19086

[FrameworkBundle] add "mapping" configuration key at validation secti… #19086

Conversation

davewwww commented Jun 17, 2016 • edited Loading

GromNaN commented Jun 17, 2016 • edited Loading

linaori commented Jun 17, 2016

davewwww commented Jun 17, 2016 • edited Loading

linaori commented Jun 17, 2016

davewwww commented Jun 22, 2016

GuilhemN Jun 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi Jun 24, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuilhemN commented Jun 23, 2016

davewwww commented Jun 30, 2016 • edited Loading

Choose a reason for hiding this comment

xabbuh commented Jul 4, 2016

davewwww commented Jul 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lunetics commented Nov 19, 2016

davewwww commented Nov 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Jan 10, 2017

HeahDude left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Jan 20, 2017

davewwww commented Jan 20, 2017

fabpot commented Jan 20, 2017

davewwww commented Jan 20, 2017

fabpot commented Jan 22, 2017

davewwww commented Jan 23, 2017

fabpot commented Jan 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davewwww commented Jun 17, 2016 •

edited

Loading

GromNaN commented Jun 17, 2016 •

edited

Loading

davewwww commented Jun 17, 2016 •

edited

Loading

GuilhemN Jun 23, 2016 •

edited

Loading

ogizanagi Jun 24, 2016 •

edited

Loading

davewwww commented Jun 30, 2016 •

edited

Loading