This PR was merged into the 3.4 branch.
Discussion
----------
[DoctrineBridge] indexBy does not refer to attributes, but to column names
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#37982
| License | MIT
| Doc PR |
Commits
-------
af1a6208ec indexBy does not refer to attributes, but to column names
This PR was merged into the 5.x branch.
Discussion
----------
[DoctrinBridge] make Uid types stricter
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Reviewing #38600 made me realize we don't need to deal with converting strings to db values.
We should only support converting actual `AbstractUid` instances in the DB.
Also, the binary types should *not* extend `GuidType`.
Commits
-------
ba31d0ee59 [DoctrinBridge] make Uid types stricter
This PR was squashed before being merged into the 5.x branch.
Discussion
----------
[RateLimiter] Added reserve() to LimiterInterface and rename Limiter to RateLimiter
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
While Javier wrote documentation for this new component, we found a couple of confusing elements that might need some tweaks:
* The `Limiter` class (previously called `LimiterFactory`) has imho a bit strange name, as it's not a limiter and it doesn't implement `LimiterInterface`. It can only new limiters. I believe `LimiterFactory` - like `LockFactory` - would be the most clear, but as that was rejected before, here is another proposal using `RateLimiter`.
* `reserve()` was now only part of the token bucket implementation. That made it a bit less useful. I think I've found a way to also allow reserving future hits in the fixed window implementation, so I've moved it to the `LimiterInterface`.
Commits
-------
cd34f21254 [RateLimiter] Added reserve() to LimiterInterface and rename Limiter to RateLimiter
This PR was merged into the 5.x branch.
Discussion
----------
[Lock][Semaphore] Add Factory::createFromKey and deprecate lock.store services
I| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | yes
| Tickets | /
| License | MIT
| Doc PR | todo
The `lock` service and aliases have been deprecated in #38576. This PR also deprecate the `lock.store` service and aliases.
People should always inject the factory into their services, except for one scenario: When they want building the `Lock` manually because they want to keep the key.
This scenario require declaring `lock.store` service and `PersisingStoreInterface` aliases.
This could be avoided if people had a method to create a lock from a given key.
This PR adds a `LockFactory::createFromKey()` method and deprecate all `lock.store` services
I also updated the Semaphore component for consistency, and helping people to only inject the SemahoreFactory.
nb: use cases for serializing the keys:
- Netflix allows only 5 users of the same family at the same time.
- An holiday apartment rental avoid users putting the same apartment in the basket
- ...
Commits
-------
91fa3e311d Deprecate lock.store aliases
This PR was merged into the 5.x branch.
Discussion
----------
[HttpClient] added `extra.trace_content` option to `TraceableHttpClient` to prevent it from keeping the content in memory
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
`TraceableHttpClient` leaks memory by definition. But sometimes, this leak is to important, especially when keeping the response content in memory.
This PR adds a new `trace_content` option under `extra` so that consumers can tell the client to not trace the content. This will be ignored when `TraceableHttpClient` is not in use.
Commits
-------
61290d5aa4 [HttpClient] added `extra.trace_content` option to `TraceableHttpClient` to prevent it from keeping the content in memory
This PR was merged into the 5.x branch.
Discussion
----------
[Console] Don't register signal handlers if pcntl is disabled
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38496
| License | MIT
| Doc PR | todo
This PR skips the default signal registration when pcntl is not installed or disabled via the `disable_functions` INI directive (which is common for prod infrastructures).
When registering a `SignalableCommand`, a clear exception is thrown.
Best reviewed [without whitespaces](https://github.com/symfony/symfony/pull/38589/files?w=1)
Commits
-------
8fe876341e [Console] Don't register signal handlers if pcntl is disabled
This PR was merged into the 5.x branch.
Discussion
----------
[FrameworkBundle] Missing Changelog for deprecating services
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | yes
| Tickets | /
| License | MIT
| Doc PR | /
I forgot to add CHANGELOG/UPGRADE when deprecating alias `lock` in #38576. (thank you @nicolas-grekas for reminding this)
This PR also adds a missing alias registered for argument.
Commits
-------
f9ddc5c147 Missing Changelog for deprecating services
This PR was merged into the 5.x branch.
Discussion
----------
[Semaphore] Reset Key lifetime time before we acquire it
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | /
This is the same issue fixed by #38553 but for Semaphore component.
Commits
-------
e7ffd5d2e7 Reset Key lifetime time in semaphore
This PR was squashed before being merged into the 5.x branch.
Discussion
----------
[RateLimiter] Adding SlidingWindow algorithm
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? |
| Tickets |
| License | MIT
| Doc PR | Should be added
This is a draft PR to make sure we like the idea. I'll keep working on adding tests.
Commits
-------
c6d3b70315 [RateLimiter] Adding SlidingWindow algorithm
This PR was merged into the 5.x branch.
Discussion
----------
Deeprecate lock service
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | yes
| Tickets | #38458
| License | MIT
| Doc PR | TODO
This PR deprecate the `lock`, `LockInterface` service ass discussed in #38458
Commits
-------
40ea90ef6b Deeprecate lock service
This PR was merged into the 3.4 branch.
Discussion
----------
[HttpFoundation] Fix Range Requests
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | #38295
| License | MIT
| Doc PR |
This PR fixes some deviations from [RFC 7233](https://tools.ietf.org/html/rfc7233) for handling range requests, mentioned in #38295.
- overlapping ranges are now satisfiable (e.g. when requested range end is larger than the file size)
- range units other than `bytes` will get ignored
- range requests for methods other than `GET` will be ignored
I did not manage yet to implement the support for multiple ranges, but also don't know, if that's needed here.
Commits
-------
681804ba1a [HttpFoundation] Fix Range Requests
* 5.1:
[Contracts] add branch-aliases for dev-main
[Cache] Make Redis initializers static
[Messenger] Fixed typos in Connection
[CI] Fixed build on AppVeyor
Fix tests typo
[Lock] Reset Key lifetime time before we acquire it
[CI] Silence errors when remove file/dir on test tearDown()
Fix tests
Remove content-type check on toArray methods
* 4.4:
[Contracts] add branch-aliases for dev-main
[Cache] Make Redis initializers static
Fix tests typo
[Lock] Reset Key lifetime time before we acquire it
[CI] Silence errors when remove file/dir on test tearDown()
This PR was squashed before being merged into the 5.x branch.
Discussion
----------
Fix minor issue when sharing windows between Limiters
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
If I start using my custom Limiter, then change back to `FixedWindowLimiter`, then my cache might contain a value that `FixedWindowLimiter` does not support.
This PR makes sure that we handle such switch.
Commits
-------
e9ac9712d8 Fix minor issue when sharing windows between Limiters
This PR was squashed before being merged into the 5.x branch.
Discussion
----------
[Messenger][Redis] Adding support for lazy connect
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#38558
| License | MIT
| Doc PR | Should be added
With inspiration from the CacheComponent. This PR makes it possible to make the connection to Redis only when you first use it.
Commits
-------
1d7c8013e6 [Messenger][Redis] Adding support for lazy connect
This PR was merged into the 4.4 branch.
Discussion
----------
[Lock] Reset Key lifetime time before we acquire it
| Q | A
| ------------- | ---
| Branch? | 5.1 (maybe lower, I'll check)
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38541
| License | MIT
| Doc PR | n/a
Im out on somewhat deep water now. I am pretty sure we should reset the Key lifetime every time we acquire it. Without it it will me tricky to re-use a lock. (As pointed out by #38541)
@jderusse can you confirm.
Commits
-------
55ad70225a [Lock] Reset Key lifetime time before we acquire it
This PR was merged into the 4.4 branch.
Discussion
----------
Fix tests typo
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
While working on PSR 16 cache tests, I found this small glitch in the mocked `isHit` method
Commits
-------
047ce05f6b Fix tests typo
This PR was squashed before being merged into the 5.x branch.
Discussion
----------
[FrameworkBundle] Bugfixes in buildDir in the CacheClear command
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38547
| License | MIT
| Doc PR | n/a
Making sure one can clear cache with and without a buildDir
Commits
-------
2cad6bbbc7 [FrameworkBundle] Bugfixes in buildDir in the CacheClear command
This PR was merged into the 3.4 branch.
Discussion
----------
Remove "version" from composer.json files, use "branch-version" instead
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Waiting for confirmation from @Seldaek or @naderman
Commits
-------
f9ed6940fd Remove "version" from composer.json files, use "branch-version" instead
* 5.1:
[DI] fix dumping env vars
[HttpClient] skip executing the multi handle when it's freed already
[HttpClient] fix using freed curl resource at destruct time
[HttpClient] shutdown verbose output from curl at destruction
fix warning for preloading TranslatorTrait class
[Typography] Remove unneeded description comments
* 4.4:
[DI] fix dumping env vars
[HttpClient] skip executing the multi handle when it's freed already
[HttpClient] fix using freed curl resource at destruct time
[HttpClient] shutdown verbose output from curl at destruction
fix warning for preloading TranslatorTrait class
[Typography] Remove unneeded description comments
This PR was merged into the 5.x branch.
Discussion
----------
[HttpClient] simplify retry mechanism around RetryStrategyInterface
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Replaces #38466
I feel like the mechanism behind RetryableHttpClient is too bloated for no pragmatic reasons.
I propose to merge RetryDeciderInterface and RetryBackoffInterface into one single RetryStrategyInterface.
Having two separate interfaces supports no real-world use case. The only implementations we provide are trivial, and they can be reused by extending the provided GenericRetryStrategy implementation if one really doesn't want to duplicate that.
The methods are also simplified by accepting an AsyncContext as argument. This makes the signatures shorter and allows accessing the "info" of the request (allowing to decide based on the IP of the host, etc).
/cc @jderusse
Commits
-------
544c3e8678 [HttpClient] simplify retry mechanism around RetryStrategyInterface
This PR was merged into the 5.x branch.
Discussion
----------
[Security] Making login link signature_properties option required
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes (for a 5.2 feature)
| New feature? | no
| Deprecations? | no
| Tickets | none
| License | MIT
| Doc PR | not needed
Hi!
My intention was always to force the user to set this option. Before this PR, you can simply leave this option off completely without a validation error. Thanks to Wouter for finding it.
Also: made some punctuation & capitalization consistent on info messages.
Cheers!
Commits
-------
f7bb954979 Making login link signature_properties option required
This PR was merged into the 5.x branch.
Discussion
----------
[Serializer] Enabled mapping configuration via attributes
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | N/A
| License | MIT
| Doc PR | TODO
Let's use attributes for the serializer mapping configuration!
```php
class MyEntity
{
#[Groups(['list', 'detail])]
#[SerializedName('some-property')]
public $someProperty;
#[Ignore]
public string $secret;
}
```
Commits
-------
cfb9986203 [Serializer] Enabled mapping configuration via attributes.
This PR was merged into the 5.x branch.
Discussion
----------
[Form] fix ViolationMapper was always generating a localized label for each FormType
| Q | A
| ------------- | ---
| Branch? | 5.x (fix new behavior from 5.2)
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | /
Follow-up of #38435, with branch changed
**Explanation of the issue**
In Symfony 5.2, the `{{ label }}` placeholder can be used in constraint messages (-> introduced in commit 0d9f44235c)
However, the way it was coded is introducing a small side effect: now, every time there is validation error, `ViolationMapper` will ask the Form Type its `label`, and if not `false`, it will try to translate it.
**Why it is important/why it causes a BC break**
Since by default `AbstractType` does not have any `label`, it also introduces a minor BC break.
I will explain it with an example: in a project I work on, we check we don't have any missing translation. Sometimes we have violation errors bound to form ; then current code will get Form `label`, which in `null` in form type classes (which is quite usual I believe), so it will generate one, and pass that one to translator. And we see a lot on erroneous missing translations.
**Proposed fix**
This fix moves all this logic into a `if`, so `ViolationMapper` call the translator component only if `{{ label }}` placeholder is used in constraint error message.
On top of fixing BC, it has the benefit of lowering the performance cost for every violation when the feature is not used.
I added a test, as I believe the behavior should be guaranteed from now on.
Commits
-------
aee5571a71 [Form] fix ViolationMapper was always generating a localized label for each FormType
This PR was merged into the 5.x branch.
Discussion
----------
[Mailer] Fix mailjet image embedding
Filename is not enough to embed the image through cid, ContentID field
has to be set with the cid identifier used in the HTMLPart
| Q | A
| ------------- | ---
| Branch? | 5.1 <!-- see below -->
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | none <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | not needed <!-- required for new features -->
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.
Additionally (see https://symfony.com/releases):
- Always add tests and ensure they pass.
- Never break backward compatibility (see https://symfony.com/bc).
- Bug fixes must be submitted against the lowest maintained branch where they apply
(lowest branches are regularly merged to upper ones so they get the fixes too.)
- Features and deprecations must be submitted against branch 5.x.
-->
Hello!
While switching from `Mandrill` to `Mailjet` I noticed that embedding images was not working properly, the `Content-ID` mime is not set properly without setting the `ContentID` key. With this change things will work as described in the [symfony documentation](https://symfony.com/doc/current/mailer.html#embedding-images)
[Mailjet reference](https://dev.mailjet.com/email/guides/send-api-v31/#send-with-attached-files)
Commits
-------
7ab772eeea [Mailer] Fix mailjet image embedding
* 5.1:
Disable the PhpUnit bridge when testing it
[PropertyInfo] Support for the mixed type.
Don't unset the inflate resource on close as it might still be needed
[HttpClient] Fix CurlHttpClient memory leak
[Form] Add Bosnian (bs) validators translation
[Form] Add missing Serbian (latn & cyrl) validators translation
[Cache] skip igbinary < 3.1.6
[Ldap] Bypass the use of `ldap_control_paged_result` on PHP >= 7.3
[Form] [Validator] added pt_BR translations
Estonian update
Fix no collection in extract default value
[PhpUnitBridge] fix running parallel tests with phpunit 9
[VarDumper] fix truncating big arrays
* 4.4:
Disable the PhpUnit bridge when testing it
[PropertyInfo] Support for the mixed type.
Don't unset the inflate resource on close as it might still be needed
[HttpClient] Fix CurlHttpClient memory leak
[Form] Add Bosnian (bs) validators translation
[Form] Add missing Serbian (latn & cyrl) validators translation
[Cache] skip igbinary < 3.1.6
[Ldap] Bypass the use of `ldap_control_paged_result` on PHP >= 7.3
[Form] [Validator] added pt_BR translations
Estonian update
[PhpUnitBridge] fix running parallel tests with phpunit 9
[VarDumper] fix truncating big arrays
* 3.4:
Disable the PhpUnit bridge when testing it
[PropertyInfo] Support for the mixed type.
[Form] Add Bosnian (bs) validators translation
[Form] Add missing Serbian (latn & cyrl) validators translation
[Form] [Validator] added pt_BR translations
Estonian update
[PhpUnitBridge] fix running parallel tests with phpunit 9
[VarDumper] fix truncating big arrays
This PR was squashed before being merged into the 5.x branch.
Discussion
----------
[Validator] Upgraded constraints to enable named arguments and attributes
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | #38096
| License | MIT
| Doc PR | TODO with symfony/symfony-docs#14305
This PR enables all remaining atomic (!= composite) constraints to be used as attributes.
The only exception is `UniqueEntity` from Doctrine bridge because we don't have a Doctrine ORM release yet that supports PHP 8. So I could migrate that one as well, but I cannot really test it.
Commits
-------
fb99eb2052 [Validator] Upgraded constraints to enable named arguments and attributes
This PR was merged into the 5.x branch.
Discussion
----------
[Security][Login Link] Allow null and DateTime objects to be used as signatureProperties
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Returning `DateTime` objects seems like a common use-case to automatically expire all login links when one is used or to only allow the login link to be used once.
**Before**
```php
class User
{
private ?\DateTime $lastAuthenticatedAt = null;
// ...
public function getLastAuthenticatedAtString(): string
{
return null === $this->lastAuthenticatedAt ? '' : $this->lastAuthenticatedAt->format('c');
}
}
```
```yaml
security:
firewalls:
main:
login_link:
# ...
signature_properties: ['lastAuthenticatedAtString']
````
**After**
```php
class User
{
private ?\DateTime $lastAuthenticatedAt = null;
// ...
public function getLastAuthenticatedAt(): ?\DateTime
{
return $this->lastAuthenticatedAt;
}
}
```
```yaml
security:
firewalls:
main:
login_link:
# ...
signature_properties: ['lastAuthenticatedAt']
````
---
The disadvantage of this patch is that there needs to be some boundary as to which objects we want to support casting to a scalar, but I'm convinced that `DateTime` objects will commonly be used as signature properties.
cc @weaverryan
Commits
-------
0f947b2e84 Allow null and DateTime objects to be used as signatureProperties
This PR was merged into the 5.x branch.
Discussion
----------
[Security] Add error message when using LoginLinkHandler outside a firewall
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Add a more friendly error message when autowiring `LoginLinkHanderInterface` in a route outside the firewall. Current error was `Call to a member function getName() on null`.
Commits
-------
f807b5fc15 Add error message when using LoginLinkHandler outside a firewall
Small mistake in the rate limiter configuration, instead of unsetting the `storage_service` option the never existing `storage` option was unset, resulting into an application error when trying to use a Limiter in your application.
Uncaught PHP Exception: The option "storage_service" does not exist. Defined options are: "id", "interval", "limit", "rate", "strategy"."
Returning DateTime objects seems like a common use-case to automatically expire
all login links when one is used or to only allow the login link to be used
once.
This PR was merged into the 5.x branch.
Discussion
----------
[Notifier] Introduce NullMessage and remove transport setter in MessageInterface
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | - <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | - <!-- required for new features -->
Follow-up PR of https://github.com/symfony/symfony/pull/36479
Commits
-------
5701e89960 Introduce NullMessage and remove transport setter in MessageInterface
This PR was merged into the 5.x branch.
Discussion
----------
[lock] Mark Key unserializable whith PgsqlStore
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | /
Marks key unserializable #38395 with the new PgsqlStore #38346
Commits
-------
eb934e9015 Mark Key unserializable whith PgsqlStore
This PR was merged into the 5.x branch.
Discussion
----------
[SecurityBundle] Make user lazy loading working without user provider
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix#38429 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
Make user lazy loading in security working again without user provider.
Commits
-------
df9e8486f5 Make user lazy loading working without user provider
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[Ldap] Bypass the use of `ldap_control_paged_result` on PHP >= 7.3
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38352
| License | MIT
| Doc PR |
As stated on #38352 [ldap_control_paged_result](https://www.php.net/manual/en/function.ldap-control-paged-result.php) and [ldap_control_paged_result_response](https://www.php.net/manual/en/function.ldap-control-paged-result-response.php) have been deprecated since PHP 7.4 and will be removed on PHP 8.0.
With this fix, Query uses serverctrls to handle LDAP results pagination.
Since `serverctrls` where introduced in PHP 7.3 and they are the only way to circumvent the usage of `ldap_control_paged_result`, I've added a new Query class implementation which uses `serverctrls` to control pagination.
To do so I've also had to update the LDAP Adapter in order to use the new class if PHP version 7.3 or greater are found
Commits
-------
d332b30526 [Ldap] Bypass the use of `ldap_control_paged_result` on PHP >= 7.3
This PR was merged into the 5.x branch.
Discussion
----------
[HttpClient] Add jitter to RetryBackoff
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | TODO
From the idea https://twitter.com/mtdowling/status/1313205613158043648 this PR adds a new `jitter` parameter to the ExponentialBackOff implementation.
jitter is a percentage (float between 0 and 1) of randomness to apply to the computed delay.
ie. if the initial delay is 1000ms, and jitter=0.2, the finale delay will be an number between 800 and 1200 (1000 +/- 20%)
Commits
-------
ace731437e Add jitter to RetryBackof
This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
[Form] [Validator] added pt_BR translations
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | --
| License | MIT
| Doc PR | --
Added missing pt_BR translations to Form and Validator components.
Commits
-------
4bede2824c [Form] [Validator] added pt_BR translations
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[Mime] Fix serialization of RawMessage
| Q | A
| ------------- | ---
| Branch? | 4.4 <!-- see below -->
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix#38430, Related #33394 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | - <!-- required for new features -->
The serialization of RawMessage is currently broken if using a generator for message like done by `Symfony\Component\Mailer\SentMessage` see 5f1c3a7972/src/Symfony/Component/Mailer/SentMessage.php (L45)
This patch converts the message to a string so further serialization can be done.
This patch probably also solves #33394.
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.
Additionally (see https://symfony.com/releases):
- Always add tests and ensure they pass.
- Never break backward compatibility (see https://symfony.com/bc).
- Bug fixes must be submitted against the lowest maintained branch where they apply
(lowest branches are regularly merged to upper ones so they get the fixes too.)
- Features and deprecations must be submitted against branch 5.x.
-->
Commits
-------
fd99eb26d8 [Mime] Fix serialization of RawMessage
This PR was merged into the 5.2-dev branch.
Discussion
----------
[DoctrineBridge] fix and replace namespace to Uid
| Q | A
| ------------- | ---
| Branch? | master <!-- see below -->
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | https://github.com/symfony/symfony/pull/37678#discussion_r499709057 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | ... <!-- required for new features -->
This post should also be corrected: https://symfony.com/blog/new-in-symfony-5-2-doctrine-types-for-uuid-and-ulid cc @javiereguiluz
Commits
-------
28d1169714 [DoctrineBridge] fix and replace namespace to Uid
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Validator] Migrate File and Image constraints to attributes
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | #38096
| License | MIT
| Doc PR | TODO with symfony/symfony-docs#14305
I have migrated a lot of the constraints already and am preparing a big PR with them at the moment. I decided to pull this part out because it might raise some discussion.
This PR enables the `File` and `Image` constraints to be used as attributes. Especially the constructor signature of the `Image` constraint has grown pretty large this way. This by itself should be a big problem, if we don't expect the constructor to be called with ordered parameters by userland code. But it shows that the constraints have grown a bit too large. We might want to consider to split it.
Commits
-------
d8c186938e [Validator] Migrate File and Image constraints to attributes.
This PR was merged into the 5.2-dev branch.
Discussion
----------
[HttpClient] minor fixes in RetryableHttpClient
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Commits
-------
495562836a [HttpClient] minor fixes in RetryableHttpClient
This PR was merged into the 3.4 branch.
Discussion
----------
Fix type annotation in ExpressionLanguage\Token
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
The expected argument `$type` should be a string - the strict comparison would always fail with the current annotated types (`array|int`).
See the constructor + constants for reference:
7db7dcc431/src/Symfony/Component/ExpressionLanguage/Token.php (L33)7db7dcc431/src/Symfony/Component/ExpressionLanguage/Token.php (L25-L30)
Commits
-------
bfde15b728 Fix type annotation
* 5.1:
Added Stopwatch example to the README
Bump Symfony version to 5.1.8
Update VERSION for 5.1.7
Update CHANGELOG for 5.1.7
Bump Symfony version to 4.4.16
Update VERSION for 4.4.15
Update CHANGELOG for 4.4.15
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Validator] Use comparison constraints as attributes
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | yes
| Tickets | #38096
| License | MIT
| Doc PR | TODO, let's add it to symfony/symfony-docs#14305
This PR enables all child classes of `AbstractComparison` to be used as attributes.
Some of those constraints used a trait called `NumberConstraintTrait` for a shared implementation. After my changes, that trait did not fit well anymore, so I've added a new `ZeroComparisonConstraintTrait` as a replacement. Although I don't expect `NumberConstraintTrait` to provide much value outside of the Symfony codebase, I think we cannot safely change it because it was not labelled as `@internal`. This is basically why I went for the deprecation.
Commits
-------
b5bdf8288f [Validator] Use comparison constraints as attributes.
This PR was squashed before being merged into the 5.2-dev branch.
Discussion
----------
[HttpFoundation] Expired cookies string representation consistency & tests
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| License | MIT
These changes add consistent behavior when converting expired cookies back and forth from string representation into `Symfony\Component\HttpFoundation\Cookie` instances in `Cookie::fromString`:
- When `Max-Age` is zero and `expires` is in the past, the `expires` date is kept as is (previous behavior: `expires` is overwritten with current timestamp because it is reset to current timestamp + `Max-Age`)
- When `Max-Age` is zero and `expires` is in the future, expires is reset to current timestamp, as `Max-Age` is the preferred "source of truth" (same as previous behavior)
- Add tests for how the Cookie class handles `Max-Age` in a cookie string and how `expires` and `Max-Age` interact
- Extract helper function `expiresTimestamp` so converting to a unix timestamp can be reused in `Cookie::fromString`
This is more a new feature than a bug fix in my mind, therefore I would include it in 5.1+.
Commits
-------
4f5d5eceb0 [HttpFoundation] Expired cookies string representation consistency & tests
This PR was merged into the 5.2-dev branch.
Discussion
----------
[lock] Prevent user serializing the key when store does not support it.
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | /
Some store relies on connection with the running process. ie. kernel relaease flock/semaphore, or zookeeper neeeds a connection to the database.
When the users tries to serialize the key to send it to another process, they are not aware that they lose the lock.
This PR throws an exception in that situation.
Commits
-------
1ec0630262 Prevent user serializing the key
This PR was merged into the 5.2-dev branch.
Discussion
----------
Remove array return type from Request::toArray()
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38400
| License | MIT
| Doc PR | -
Laravel already extends Symfony's `Request` class and defines it's own `toArray` method. https://github.com/symfony/symfony/pull/38224 added a new `toArray` method to this class with a different signature to the one that is in Laravel, causing fatal errors (https://github.com/laravel/framework/issues/34660). I think the best course of action here is to remove the return type for now, and only add it in Symfony 6. This will allow Symfony 6.0 and Laravel 11 to synchronize adding the return type.
Older versions of Laravel can't just change their signature to add an array return type to them, because that would be a breaking change for Laravel users extending Laravel's request class. I'm thinking, in particular, API packages and the like, or just straight up application code.
Commits
-------
8b291a49a6 Remove array return type from Request::toArray()
This PR was merged into the 5.1 branch.
Discussion
----------
Handle consecutive supports() calls in the RememberMeAuthenticator
| Q | A
| ------------- | ---
| Branch? | 5.1
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38206
| License | MIT
| Doc PR | -
If I read the issue correctly, the problem is not so much that `autoLogin()` is called in supports, but that it is called multiple times in the same request (in lazy firewalls). This is fixed by this issue.
@qurben or @fancyweb do you have an application with this error, and can you please test the patch in this PR? Please let me know if this actually fixed the issue. (if you can't, I'll create a small demo app to test this one)
Commits
-------
e0d1867b54 Handle consecutive supports() calls in the RememberMeAuthenticator
This PR was merged into the 5.2-dev branch.
Discussion
----------
[HttpClient] Minor fix of type and message in ExponentialBackOff
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Make the type consistent and fix an error message
Commits
-------
6149a0c04b [HttpClient] Minor fix of type and message in ExponentialBackOff
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Console] Remove "php" invokation from help messages.
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| License | MIT
Discusstion started here:
https://github.com/symfony/symfony/pull/38349
I was a bit puzzled to find that the help for the list and help commands suggests that you call the console application by prefixing it with `php myconsoleapp`.
As suggested in the PR above I am removing the `php ` prefix from the help.
I am providing a script with a shebang like the first example suggested in the following link:
https://symfony.com/doc/current/components/console.html
Eventually I want to distribute my console app as docker image so there is no need for php installed or the users even knowing is written in php.
The script name is easy to override by just setting a different value to `$_SERVER['PHP_SELF']` but this php prefix is hardcoded into the help strings for the the two default commands available.
Slightly related to #38347 as I am trying to improve the console help output.
Commits
-------
e036c30e7a [Console] Remove "php" invokation from help messages.
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Form] Implement Twig helpers to get field variables
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | https://github.com/symfony/symfony-docs/pull/14308
Designing Symfony Forms has always been difficult, especially for developers not comfortable with Symfony or Twig. The reason behind this difficulty is that the current `form_*` helper functions, while providing a way to quickly render a form, are hiding the generated HTML behind a notation specific to Symfony.
HTML standards introduced many new attributes since the Form component was created, from new constraints to how should inputs be displayed, treated by screen readers, etc.
I propose to introduce a series of new Twig functions to help create more flexible forms without the hurdle of having to use `form_*` functions. I called these methods `field_*` because they aim at rendering only the tiny bits of strings necessary to map forms to the Symfony backend.
The functions introduced are:
* `field_name` returns the name of the given field
* `field_value` returns the current value of the given field
* `field_label` returns the label of the given field, translated if possible
* `field_help` returns the help of the given field, translated if possible
* `field_errors` returns an iterator of strings for each of the errors of the given field
* `field_choices` returns an iterator of choices (the structure depending on whether the field uses or doesn't use optgroup) with translated labels if possible as keys and values as values
A quick example of usage of these functions could be the following:
``` twig
<input
name="{{ field_name(form.username) }}"
value="{{ field_value(form.username) }}"
placeholder="{{ field_label(form.username) }}"
class="form-control"
/>
<select name="{{ field_name(form.country) }}" class="form-control">
<option value="">{{ field_label(form.country) }}</option>
{% for label, value in field_choices(form.country) %}
<option value="{{ value }}">{{ label }}</option>
{% endfor %}
</select>
<select name="{{ field_name(form.stockStatus) }}" class="form-control">
<option value="">{{ field_label(form.stockStatus) }}</option>
{% for groupLabel, groupChoices in field_choices(form.stockStatus) %}
<optgroup label="{{ groupLabel }}">
{% for label, value in groupChoices %}
<option value="{{ value }}">{{ label }}</option>
{% endfor %}
</optgroup>
{% endfor %}
</select>
{% for error in field_errors(form.country) %}
<div class="text-danger mb-2">
{{ error }}
</div>
{% endfor %}
```
There are several advantages to using these functions instead of their `form_*` equivalents:
* they are much easier to use for developers not knowing Symfony: they rely on native HTML with bits of logic inside, instead of relying on specific tools needing to be configured to display proper HTML
* they allow for better integration with CSS frameworks or Javascript libraries as adding a new HTML attribute is trivial (no need to look at the documentation)
* they are easier to use in contexts where one would like to customize the rendering of a input in details: having the label as placeholder, displaying a select empty field, ...
The `form_*` functions are still usable of course, but I'd argue this technique is actually easier to read and understand.
Commits
-------
3941d70928 [Form] Implement Twig helpers to get field variables
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[Lock] Fix StoreFactory to accept same DSN syntax as AbstractAdapter
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#35350
| License | MIT
| Doc PR |
Updates `Symfony\Component\Lock\Store\StoreFactory` to accept same DSN syntax as `Symfony\Component\Cache\Adapter\AbstractAdapter` which is used to create Redis class instance.
Commits
-------
4ebbe3d86b [Lock] Fix StoreFactory to accept same DSN syntax as AbstractAdapter
This PR was squashed before being merged into the 5.2-dev branch.
Discussion
----------
[Security] Magic login link authentication
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | none
| License | MIT
| Doc PR | TODO
Hi!
This adds a Slack-style "magic link" login authenticator to the new login system: (A) enter your email into a form, (B) receive an email with a link in it (C) click that link and you are authenticated!
For most users, implementing this would require:
A) Create a [controller](https://github.com/weaverryan/symfony-magic-login-link-example/blob/master/src/Controller/MagicLinkLoginController.php) with the "enter your email" form and a route for the "check" functionality (similar to `form_login`)
B) Activate in `security.yaml`:
```yml
security:
enable_authenticator_manager: true
# ...
firewalls:
# ...
main:
# ...
login_link:
check_route: 'magic_link_verify'
# this is an important and powerful option
# An array of properties on your User that are used to sign the link.
# If any of these change, all existing links will become invalid
# tl;dr If you want the modification of ANY field to invalidate ALL existing magic links immediately,
# then you can add it to this list. You could even add a "lastLoginLinkSentAt" to invalid
# all existing login links when a new one is sent.
signature_properties: [id, password, email]
# optional - by default, links can be reused but have a 10 minute lifetime
#max_uses: 3
#used_link_cache: cache.app
```
Done! This will generate a URL that looks something like this:
> https://127.0.0.1:9033/login/verify?user=weaverryan@gmail.com&expires=1601342578&hash=YzE1ZDJlYjM3YTMyMjgwZDdkYzg2ZjFlMjZhN2E5ZWRmMzk3NjAxNjRjYThiMjMzNmIxYzAzYzQ4NmQ2Zjk4NA%3D%3D
We would implement a Maker command this config + login/controller. The implementation is done via a "signed URL" and an optional cache pool to "expire" links. The hash of the signed URL can contain any user fields you want, which give you a powerful mechanism to invalidate magic tokens on user data changes. See `signature_properties` above.
#### Security notes:
There is a LOT of variability about how secure these need to be:
* A) Many/most implementation only allow links to be used ONE time. That is *possible* with this implementation, but is not the *default*. You CAN add a `max_uses` config which stores the expired links in a cache so they cannot be re-used. However, to make this work, you need to do more work by adding some "page" between the link the users clicks and *actually* using the login link. Why? Because unless you do this, email clients may follow the link to "preview" it and will "consume" the link.
* B) Many implementations will invalidate all other login links for a user when a new one is created. We do *not* do that, but that IS possible (and we could even generate the code for it) by adding a `lastLoginLinkSentAt` field to `User` and including this in `signature_properties`.
* C) We *do* invalidate all links if the user's email address is changed (assuming the `email` is included in `signature_properties`, which it should be). You can also invalidate on password change or whatever you want.
* D) Some implementations add a "state" so that you can only use the link on the same device that created it. That is, in many cases, quite annoying. We do not currently support that, but we could in the future (and the user could add it themselves).
Thanks!
#### TODOS:
* [x] A) more tests: functional (?) traits
* [ ] B) documentation
* [ ] C) MakerBundle PR
* [ ] D) Make sure we have what we need to allow that "in between" page
* [ ] E) Create a new cache pool instead of relying on cache.app?
Commits
-------
a8afe109d8 [Security] Magic login link authentication
This PR was squashed before being merged into the 5.2-dev branch.
Discussion
----------
[HttpFoundation] Add Request::toArray() for JSON content
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | ni
| New feature? | yes
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
The past few months I've been working more and more with javascript and APIs. I've written controllers that parse json from the request body. I've found myself copying code between projects so I looked at the possibility to add this code to the `Request` class itself.
### Usage
```http
POST /add-user
Content-Type: application/json
{"name": "Tobias", "email": "tobias.nyholm@gmail.com"}
```
```php
use Symfony\Component\HttpFoundation\Exception\JsonException;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\JsonResponse;
class MyController
// ...
public function addUser(Request $request): JsonResponse
{
try {
$data = $request->toArray();
} catch (JsonException $e) {
return new JsonResponse(['message'=>'Request does not contain valid json'], 415);
}
$command = new AddUser($data['email'] ?? '', $data['name'] ?? '');
try {
$this->commandBus->dispatch($command);
} catch (ValidationFailedException $e) {
return new JsonResponse(['message' => 'Unexpected JSON body'], 400);
}
return new JsonResponse(['message' => 'User successfully added']);
}
}
```
----------
I've searched but I have not found that this has been proposed before.. With is strange.. ¯\\_(ツ)_/¯
Commits
-------
83c1a2666d [HttpFoundation] Add Request::toArray() for JSON content
This PR was merged into the 5.2-dev branch.
Discussion
----------
[PropertyInfo] Fix failure over array<string,mixed> PhpDoc type
| Q | A
| ------------- | ---
| Branch? | master (issue introduced in master)
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | /
**Background**
Since a few days ago ([this commit](80e5a7fbd8)), PropertyInfo 5.2 component supports array PhpDoc types, such as `array<string,string>`.
While testing SF 5.2 over my project, it was failing over:
```php
class SomeDoctrineEntityProxy implements \Doctrine\ORM\Proxy\Proxy
{
/**
* @var array<string, mixed> default values of properties to be lazy loaded, with keys being the property names
*
* @see \Doctrine\Common\Proxy\Proxy::__getLazyProperties
*/
public static $lazyPropertiesDefaults = array();
}
```
Upon investigation, it appears `array<string,mixed>` is causing the issue: `mixed` type is not handled.
**Expected behavior**
PropertyInfo component should not guess anything for `mixed` type (return `NULL`), but it should not throw any exception.
**Comment about my fix**
I added a test case (`arrayOfMixed`) you can use to reproduce the issue.
Since array PhpDoc types were not tested at all, I also added a test case over `array<string,string>`.
Commits
-------
590177ff8e [PropertyInfo] Fix failure over array<string,mixed> PhpDoc type
This PR was merged into the 5.2-dev branch.
Discussion
----------
[HttpClient] Fix exception triggered with AsyncResponse
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | https://github.com/symfony/symfony/pull/38289#issuecomment-702573975
| License | MIT
| Doc PR | /
When a request is replayed with the AsyncResponse and the 2nd request failed, users get an exception even when they pass `$throw=false`.
In fact, there are 2 exceptions:
1. Cannot get the content of the response twice: buffering is disabled.
73ca97c97a/src/Symfony/Component/HttpClient/Response/CommonResponseTrait.php (L51-L68)
Because CommonResponseTrait::$content is null and `self::stream` yield only the LastChunk. The content is stays null
2. HTTP/2 429 returned for "https://httpbin.org/status/429"
73ca97c97a/src/Symfony/Component/HttpClient/Response/AsyncResponse.php (L209-L225)
Because on the second request passthru is null, it didn't disable the the exception thrown on destruct for the wrapped response.
This
Commits
-------
3aedb51dd8 [HttpClient] Fix exception triggered with AsyncResponse
* 5.1:
[5.1] Ignore more deprecations for Mockery mocks
[PhpUnitBridge] Fix Deprecation file when it comes from the TestsListener
disallow FrameworkBundle 4.4+
propagate validation groups to subforms
[Form] [Validator] Add failing testcase to demonstrate group sequence issue
This PR was merged into the 5.1 branch.
Discussion
----------
[PhpUnitBridge] Fix Deprecation file when it comes from the TestsListener
| Q | A
| ------------- | ---
| Branch? | 5.1
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
https://github.com/symfony/symfony/pull/38031 caused an unwanted regression: if the frame `class` matches `SymfonyTestsListenerFor` it used to always return, now it continues so the `originClass` and `originMethod` properties are set. Therefore, the deprecation is considered as coming from the test listener. It ends up in the "legacy" group and muted because the test listeners contains the word "Legacy" in their namespaces.
Example test:
```php
<?php
namespace App\Tests;
use PHPUnit\Framework\TestCase;
use Symfony\Component\EventDispatcher\DependencyInjection\ExtractingEventDispatcher;
final class FooTest extends TestCase
{
public function test(): void
{
// ExtractingEventDispatcher is @internal
new class extends ExtractingEventDispatcher {};
$this->assertTrue(true);
}
}
```
On 4.4:
```
Other deprecation notices (1)
1x: The "Symfony\Component\EventDispatcher\DependencyInjection\ExtractingEventDispatcher" class is considered internal. It may change without further notice. You should not use it from "Symfony\Component\EventDispatcher\DependencyInjection\ExtractingEventDispatcher@anonymous".
```
On 5.1:
```
Legacy deprecation notices (1)
```
Commits
-------
1ba06a0f86 [PhpUnitBridge] Fix Deprecation file when it comes from the TestsListener
* 4.4:
disallow FrameworkBundle 4.4+
propagate validation groups to subforms
[Form] [Validator] Add failing testcase to demonstrate group sequence issue
* 3.4:
disallow FrameworkBundle 4.4+
propagate validation groups to subforms
[Form] [Validator] Add failing testcase to demonstrate group sequence issue
This PR was merged into the 3.4 branch.
Discussion
----------
[Form] propagate validation groups to subforms
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38300
| License | MIT
| Doc PR |
Commits
-------
04f5698e29 propagate validation groups to subforms
e2c7c3373d [Form] [Validator] Add failing testcase to demonstrate group sequence issue
