Commit Graph

416 Commits

Author SHA1 Message Date
Fabien Potencier
275cf8cf00 removed unused use statements 2012-11-04 09:30:21 +01:00
Pascal Borreli
a7ce6be3f8 Fixed typos 2012-10-28 23:25:34 +00:00
Fabien Potencier
7b998d9b2d Merge branch '2.1'
* 2.1:
  [ClassLoader] fixed unbracketed namespaces (closes #5747)
  slight refactoring in UrlMatcher
  [Form] Created test for DoctrineOrmTypeGuesser see #5790
  [Form] Fixed DoctrineOrmTypeGuesser to guess the "required" option for to-one associations
2012-10-27 17:59:37 +02:00
Tobias Schultze
7447ef7171 slight refactoring in UrlMatcher 2012-10-26 12:26:42 +03:00
Fabien Potencier
67d9253127 Merge branch '2.1'
* 2.1:
  added missing use statment (closes #5825)
  Code cleanup
  [WebProfilerBundle] Fixed the use of nested macros
  Removed unused use statements.
  Nsdocblocks
  [ConfigDumpReference] avoid notice for variable nodes
  fixed fallback locale
  UniqueValidatorTest, Change message on assertions
  Documented removed _form_is_choice_group function

Conflicts:
	src/Symfony/Bundle/FrameworkBundle/Command/ConfigDumpReferenceCommand.php
	src/Symfony/Bundle/WebProfilerBundle/Profiler/TemplateManager.php
2012-10-24 17:41:27 +02:00
Mario A. Alvarez Garcia
f06432bc73 Code cleanup 2012-10-22 10:37:12 -04:00
Drak
788cc2c7ef Nsdocblocks 2012-10-20 09:10:30 +02:00
Fabien Potencier
388cbff022 merged branch arnaud-lb/routing-php-dumper-simplification (PR #5734)
This PR was merged into the master branch.

Commits
-------

e54d749 [Routing] Simplified php matcher dumper (and optimized generated matcher)

Discussion
----------

[Routing] Simplified php matcher dumper (and optimized generated matcher)

Bug fix: no
Feature addition: no
Related: #3378
Backwards compatibility break: no
Symfony2 tests pass: yes

This simplifies the php matcher dumper by allowing the dumper to re-organize routes in the dumper's own structure.

As a result, dumping is made a little simpler. This is also helps much for my hostname-based routes PR #3378.

Reorganizing routes also allows to find more optimization opportunities:

Currently the dumper wraps some collections of routes in a `if (0 === strpos($pathinfo, '/someprefix')` if the collection has user-defined prefix, and if it contains more than one direct child Route. This can miss many optimization opportunities.

The PR changes this by building a prefix tree of routes based on the static prefix extracted from routes' patterns. Then every leave having a prefix and more than one child (route or collection) will be wrapped in a `if` statement.

Example:

```
// No explicit prefix is specified
@Route("/cafe")
@Route("/cacao")
@Route("/coca")
```

is compiled like this:

```
if (url starts with /c) {
    if (url starts with /ca) {
        // test route "/cafe"
        // test route "/cacao"
    }
    // test route "/coca"
}
```

Some tests have many white space changes, much more easier to review [here](https://github.com/symfony/symfony/pull/5734/files?w=1)

---------------------------------------------------------------------------

by Tobion at 2012-10-13T02:27:54Z

I'm not sure if adding these specific classes just for dumping is the best implementation because they duplicate some logic and this optimization should also work out-of-the-box with the standard RouteCollection etc.
What I have in mind is a new method in RouteCollection like `RouteCollection::optimizeTree` that returns a new RouteCollection with the Routes that includes these optimization you do here. So there would probably be no need for the new classes.

It think it requires some changes in RouteCollection like the handling of prefix that must start with a slash currently, which is too restrictive. But it should be possible.

---------------------------------------------------------------------------

by arnaud-lb at 2012-10-13T12:52:32Z

@Tobion

> I'm not sure if adding these specific classes just for dumping is the best implementation because they duplicate some logic and this optimization should also work out-of-the-box with the standard RouteCollection etc.

I think RouteCollection and DumperCollection do not share the same concerns; and RouteCollection does things that don't allow to reorganize routes freely. For instance when adding a collection to a RouteCollection this changes all the child routes' prefix, requirements, options, etc. When setting a collection's prefix, this prepends the prefix to every child route's pattern, etc.

---------------------------------------------------------------------------

by arnaud-lb at 2012-10-15T08:50:23Z

squashed the CS commits

---------------------------------------------------------------------------

by arnaud-lb at 2012-10-15T13:50:16Z

@fabpot @Tobion this PR is ready to be merged if everyone agrees

---------------------------------------------------------------------------

by Tobion at 2012-10-16T18:10:36Z

When the above is fixed, I think it's good to be merged.

---------------------------------------------------------------------------

by arnaud-lb at 2012-10-17T08:40:20Z

Fixed; thanks @Tobion @stof for your reviews

---------------------------------------------------------------------------

by Tobion at 2012-10-19T03:30:10Z

@arnaud-lb could you please test whether your PR fixes #5780 as a side-effect?
I can image that it's fixed because you use `$route->compile()->getStaticPrefix();` for prefix optimization.
If it's fixed please add a test case. If not, that's fine, and we need to fix it in another PR.
2012-10-19 07:56:40 +02:00
Fabien Potencier
88ea842c5e Merge branch '2.1'
* 2.1: (28 commits)
  Delete use of CreationExeption
  [Form] Fixed error message in PropertyPath to not advice to use a non-existing feature
  [Form] Fixed creation of multiple money fields with different currencies
  [Form] Fixed setting the "data" option to an object in "choice" and "entity" type
  Fixed Serbian plural translations.
  Fixed IPv6 Check in RequestMatcher
  Fix typo
  change what I think is a typo
  [Console] Fix error when mode is not in PATH
  [WebProfilerBundle] fixed macro usage (to be forward compatible with Twig 2.x)
  Change monolog require-dev to use the branch alias instead of dev-master
  [FrameworkBundle] partially reverted previous merge
  [2.1] Added missing error return codes in commands
  Made the router lazy when setting the context
  [WebProfilerBundle] fixed typos
  Fix incorrect variable in FileProfilerStorage
  UnitTest fix
  UnitTest fix
  added a unit test
  fixed #5384
  ...
2012-10-18 23:16:19 +02:00
Arnaud Le Blanc
e54d749d05 [Routing] Simplified php matcher dumper (and optimized generated matcher) 2012-10-16 20:50:15 +02:00
Christophe Coevoet
e0a3fc19a4 Made the router lazy when setting the context
Initializing the matcher and the generator to set the context does not make
sense as it is set anyway when building them. This avoids initializing
them in the RouterListener if you never actually use them (for instance
because you use the apache matcher).
2012-10-14 10:27:44 +02:00
Tobias Schultze
1566f9f4ad [Routing] fix handling of whitespace and synch between collection prefix and route pattern 2012-10-08 18:39:54 +02:00
Fabien Potencier
5956f9f2b4 Merge branch '2.1'
* 2.1:
  fixed CS
  added doc comments
  added doc comments
  [Validator] Updated swedish translation
  Update src/Symfony/Component/Validator/Resources/translations/validators.de.xlf
  [2.1] Exclude tests from zips via gitattributes
  [HttpKernel][Translator] Fixed type-hints
  Updated lithuanian validation translation
  [DomCrawler] Allows using multiselect through Form::setValues().
  [Translation] forced the catalogue to be regenerated when a resource is added (closes symfony/Translation#1)
  Unit test for patched method OptionsResolver::validateOptionValues().
  validateOptionValues throw a notice if an allowed value is set and the corresponding option isn't.
  [Form] Hardened code of ViolationMapper against errors
  [HttpFoundation] Fixed #5611 - Request::splitHttpAcceptHeader incorrect result order.
  [Form] Fixed negative index access in PropertyPathBuilder
  Update src/Symfony/Component/Validator/Resources/translations/validators.ro.xlf

Conflicts:
	src/Symfony/Component/DomCrawler/Form.php
	src/Symfony/Component/Process/Process.php
2012-10-06 21:57:59 +02:00
Tobias Schultze
90145d2b71 [Routing] fix handling of two starting slashes in the pattern
It would be confused with a network path e.g. '//domain/path' when generating a path so should be prevented.
2012-10-06 01:40:30 +02:00
Igor Wiedler
462dddfac9 [2.1] Exclude tests from zips via gitattributes 2012-10-04 17:17:57 +02:00
Tobias Schultze
faee47c64f added note in changelog about disabling requirements check 2012-10-03 21:52:16 +02:00
Tobias Schultze
5fbed361c2 extended phpdoc of ConfigurableRequirementsInterface 2012-10-03 21:00:23 +02:00
Tobias Schultze
1964d43823 [Routing] added test for disabled requirements check 2012-10-03 21:00:22 +02:00
Tobias Schultze
98fb915bfb [Routing] allow disabling the requirements check on URL generation 2012-10-03 21:00:21 +02:00
Fabien Potencier
7276b2d0b8 merged branch Tobion/strictrequirements2 (PR #5445)
This PR was merged into the master branch.

Commits
-------

e22823b [Routing] context params should have higher priority than defaults
16c1b01 [Routing] fixed 4 bugs in the UrlGenerator

Discussion
----------

[Routing] UrlGenerator: fixed missing query param and some ignored requirements

reopened version of #5181 (cherry-picked)

On top of that I fixed #5437 in my code and added test case.

---------------------------------------------------------------------------

by Tobion at 2012-10-03T18:41:45Z

@fabpot ping

---------------------------------------------------------------------------

by fabpot at 2012-10-03T18:43:43Z

IIUC, #5437 is a regression in 2.1 and should be done in 2.1, no?

---------------------------------------------------------------------------

by Tobion at 2012-10-03T18:46:42Z

It's not in 2.1 anymore as you reverted the PR.  #5437 is not valid currently and can be closed.
So this can either be merged in master or 2.1 whatever you prefer.
2012-10-03 20:51:18 +02:00
Tobias Schultze
94f2fb91d7 added info to routing changelog 2012-10-03 19:28:37 +03:00
Tobias Schultze
4eee88f22b [Routing] improve matching performance by using possesive quantifiers when possible (closes #5471)
My benchmarks showed a performance improvement of 20% when matching routes that make use of possesive quantifiers because it prevents backtracking when it's not needed
2012-10-03 16:41:15 +02:00
Tobias Schultze
a3147e9f13 [Routing] default requirement of variable should not disallow previous char, but the slash '/' (closes #5469) 2012-10-03 16:37:44 +02:00
Tobias Schultze
4868bec46c [Routing] fix Compiler when previous char of placeholder is no real separator (closes #5423) 2012-10-03 16:33:06 +02:00
Fabien Potencier
190ccf8dbe merged branch Tobion/compiler (PR #4225)
This PR was merged into the master branch.

Commits
-------

005a9a3 [Routing] fixed RouteCompiler for adjacent and nested placeholders

Discussion
----------

[2.2] [Routing] fixed RouteCompiler for adjacent placeholders

Tests pass: yes
Feature addition: no
Fixes: #4215
BC break: no

- Nesting placeholders works now properly, e.g. `/{foo{bar}}`. Only `bar` is a variable, the rest is static text.
- Variables without separator work now correctly too, e.g. `/{w}{x}{y}{z}.{_format}`. All variables will have the correct default regex in the current manner, that is exclude the previous static char and the next static char (which means disregarding the placeholder in between that have no seperator).

As you can see, I have not modified any tests, only added some. So this change is BC. The compiler now works under all conditions and does not fail for such patterns.

---------------------------------------------------------------------------

by Tobion at 2012-05-08T22:34:58Z

ready for review / merging
Thanks @snc for giving a helpful tip.

---------------------------------------------------------------------------

by Tobion at 2012-06-12T23:22:54Z

fabpot told me, he doesn't like PRs that both fix and enhance stuff. So I reworked this PR so that it only contains the bug fixes.
The new PR #4563 contains the feature addition.

---------------------------------------------------------------------------

by Tobion at 2012-06-26T12:33:43Z

ping @fabpot

---------------------------------------------------------------------------

by Tobion at 2012-08-14T16:29:27Z

Why wait for 2.2? It's a bugfix only, without BC break.

---------------------------------------------------------------------------

by Tobion at 2012-08-31T17:04:37Z

@fabpot I think this should go into 2.1
2012-10-03 16:28:55 +02:00
Filippo Tessarotto
45bf52358e Optimize autoload prefix in composer.json
By having more specific autoload prefixes it is possible to reduce the
number of stat calls made. Also it prevents conflicts with similar
namespaces.
2012-09-28 09:34:16 +02:00
Tobias Schultze
005a9a3c5b [Routing] fixed RouteCompiler for adjacent and nested placeholders 2012-09-10 22:41:05 +02:00
Fabien Potencier
885d473366 merged branch Tobion/empty-requirement (PR #5426)
Commits
-------

02516de [Routing] fix variable with a requirement of '0'
1f5b793 [Routing] fix setting empty requirement in Route

Discussion
----------

[Routing] fix setting empty requirement

First commit: A requirement of "^$" was overlooked and wasn't recognized as empty after stripping it in Route.
Second commit: Fixes a requirement of '0' that was ignored by the Compiler.
2012-09-10 22:27:16 +02:00
Fabien Potencier
13937de2c8 replaced self.version/2.1.* by 2.2.* in composer.json files 2012-09-10 12:59:37 +02:00
Fabien Potencier
a9a0f420e2 merged 2.1 2012-09-10 12:54:04 +02:00
Fabien Potencier
3c32fd993d replaced self.version by 2.1.* in composer.json files 2012-09-10 12:53:42 +02:00
Fabien Potencier
8b7ba9b1e5 merged branch Tobion/numeric-placeholder (PR #5439)
Commits
-------

be28e56 [Routing] disallow numeric named variables in pattern

Discussion
----------

[Routing] compile check for numeric named variables in pattern

Because PHP raises an error for such subpatterns in PCRE and thus would break matching, e.g. this is not allowed as regex `(?<123>.+)`.
So add a compile time check for a non-working pattern like '/{123}'.

---------------------------------------------------------------------------

by sstok at 2012-09-06T08:31:42Z

Strangely enough Regex buddy gives no warning or error with the pattern.
Is the name all numeric invalid or just the beginning?

1e4 and 0xFF would be perfectly valid but returns true with is_nummeric()

---------------------------------------------------------------------------

by Tobion at 2012-09-06T08:59:07Z

Any numeric is not valid. I guess this limitation is unique to PHP's binding to PCRE.
I think it's because the returned matches array of of preg_match contains both the subpattern as integer index and as named variable. So having a numeric named variable would conflict as `array['1'] === array[1]`.
2012-09-10 11:27:26 +02:00
Fabien Potencier
71b39d51de merged branch Tobion/same-requirements-issue (PR #5435)
Commits
-------

7c5cfeb [Routing] added test why #5238 is not that easy

Discussion
----------

[Routing] added test why #5238 is not that easy

This just adds a test that wasn't covered yet and shows why #5238 is not that easy as I thought at first.
2012-09-10 11:25:47 +02:00
Tobias Schultze
e22823bb7d [Routing] context params should have higher priority than defaults
fixes #5437
2012-09-07 03:30:48 +02:00
Fabien Potencier
4dc197c3e1 udpated composer.json to 2.2 2012-09-06 20:45:30 +02:00
Tobias Schultze
16c1b01531 [Routing] fixed 4 bugs in the UrlGenerator 2012-09-06 16:15:10 +02:00
Fabien Potencier
00ec9b5391 Revert "merged branch Tobion/strictrequirements (PR #5181)"
This reverts commit 2da2a44382, reversing
changes made to 58855475c6.
2012-09-05 17:44:14 +02:00
Fabien Potencier
aae4ceea56 Revert "merged branch Tobion/requirementscheck (PR #5187)"
This reverts commit 2cf50b7801, reversing
changes made to 569e29d91f.
2012-09-05 17:44:10 +02:00
Fabien Potencier
fdf25289f8 Revert "merged branch Tobion/uselessparamdefaults (PR #5400)"
This reverts commit 0f61b2eb90, reversing
changes made to 5e7723fcbb.
2012-09-05 17:44:05 +02:00
Tobias Schultze
7c5cfeb776 [Routing] added test why #5238 is not that easy 2012-09-05 06:36:05 +02:00
Tobias Schultze
02516de652 [Routing] fix variable with a requirement of '0' 2012-09-04 07:18:13 +02:00
Tobias Schultze
be28e56f4e [Routing] disallow numeric named variables in pattern
because PHP raises an error for such subpatterns in PCRE and thus would break matching, e.g. '(?<123>.+)'.
2012-09-04 07:10:18 +02:00
Tobias Schultze
1f5b7930c0 [Routing] fix setting empty requirement in Route 2012-09-04 06:31:46 +02:00
Tobias Schultze
cb7e3f5edb [Routing] added route compile check to identify a default value of a required variable that does not match the requirement 2012-08-31 08:36:21 +02:00
Tobias Schultze
33638320f2 extended phpdoc of ConfigurableRequirementsInterface 2012-08-30 16:35:30 +02:00
Tobias Schultze
5f64503132 [Routing] added test for disabled requirements check 2012-08-30 15:08:30 +02:00
Tobias Schultze
4225869939 [Routing] allow disabling the requirements check on URL generation 2012-08-30 15:08:29 +02:00
Fabien Potencier
2da2a44382 merged branch Tobion/strictrequirements (PR #5181)
Commits
-------

0706d18 [Routing] fixed 4 bugs in the UrlGenerator

Discussion
----------

[Routing] UrlGenerator: fixed missing query param and some ignored requirements

This was pretty hard to figure out. I could fix 4 bugs and refactor the code to safe 2 variables and several assignments. Sorry for doing this in one commit, but they were highly interdependent.
See the added tests for what was fixed. The most obvious bug was that a query param was ignored if it had by accident the same name as a default param (but wasn't used in the path).
In 3 cases it generated the wrong URL that wouldn't match this route. The generator wrongly ignored either the requirements or the passed parameter. I had to adjust one test that was asserting something wrong (see comments).

---------------------------------------------------------------------------

by Tobion at 2012-08-13T14:22:35Z

ping @fabpot

---------------------------------------------------------------------------

by Tobion at 2012-08-29T17:53:07Z

@fabpot I think it's important to merge this before 2.1 final.
2012-08-30 09:34:36 +02:00
Tobias Schultze
dd0403f117 [Routing] small refactoring in Router 2012-08-29 09:36:18 +03:00
Fabien Potencier
3969011f7f [Routing] fixed a test 2012-08-28 13:06:09 +02:00