2.0 Proposal

[mbabker]

See #343 for details

I'm leaving it as split-up commits to make it a little easier to see how everything evolved.

If accepted as is, what I'd personally do is this:

• Merge these PRs:
  • [x] #328
  • [x] #344
  • [x] #348
  • [ ] #350
• Cut a 1.4 release
• Merge this
• Cut a 2.0 beta

[shakaran]

@thomaspicquet Could you rebase the conflicts to keep this big one issue updated? Thanks

[deluxetom]

@mbabker I've been using your version 2.0 on production for a couple months now and its been working well ;) if that helps to get this merged :)

[shakaran]

Please @markitosgv @mbabker, could we merge this for get rid of a lot deprecation warnings annoying? The 2.0 release is taking a lot time to happen.

We need remove the deprecations and old code and release the 2.0 clean for Symfony 7.0 world. It is very annoying see the deprecations warnings from 1.0 release or master.

All the uses from

use Gesdinet\JWTRefreshTokenBundle\Generator\RefreshTokenGeneratorInterface;

Instead

use Gesdinet\JWTRefreshTokenBundle\Model\RefreshTokenManagerInterface;

Are spamming a lot project logs with deprecated.

Thanks

[mbabker]

We need remove the deprecations and old code and release the 2.0 clean for Symfony 7.0 world. It is very annoying see the deprecations warnings from 1.0 release or master.

What deprecations, specifically, are you hitting with this bundle's latest 1.x release that cannot be fixed? The bundle itself shouldn't be using its own deprecated code. If your issue is just the presence of @deprecated annotations, that alone isn't a problem (the Symfony debug loader won't randomly spit out log messages for deprecated methods when they aren't called).

All the uses from

use Gesdinet\JWTRefreshTokenBundle\Generator\RefreshTokenGeneratorInterface;

Instead

use Gesdinet\JWTRefreshTokenBundle\Model\RefreshTokenManagerInterface;

Are spamming a lot project logs with deprecated.

Is something in your project using the deprecated RefreshTokenManagerInterface::create() method? Outside of the test suite, nothing in the bundle is calling it.

[shakaran]

Is something in your project using the deprecated RefreshTokenManagerInterface::create() method? Outside of the test suite, nothing in the bundle is calling it.

it some way, when I load the bundle, always hit https://github.com/markitosgv/JWTRefreshTokenBundle/blob/master/Entity/AbstractRefreshToken.php#L16 since the autoloaders with PSR4 load all the claseses.

Since it is a interface, and it doesn't have a constructor, it is always loaded the trigger of deprecation and it should be only show when it is really in use.

This was already mentioned at https://github.com/api-platform/core/issues/4790#issuecomment-1249428021

Regarding to ::create(), I am already using createForUserWithTtl():

$refreshToken = $this->refreshTokenGenerator->createForUserWithTtl(user: $kUser, ttl: $this->ttl);

[mbabker]

it some way, when I load the bundle, always hit https://github.com/markitosgv/JWTRefreshTokenBundle/blob/master/Entity/AbstractRefreshToken.php#L16 since the autoloaders with PSR4 load all the claseses.

It's no different than trigger_deprecation() calls that live outside classes in Symfony files; files like the deprecated ContainerAwareTrait or RequestMatcher class would cause the same type of excess deprecation logging if they were arbitrarily included as well.

That deprecation being triggered isn't the autoloader arbitrarily including all files, but rather processes that eagerly scan and load files. I know API Platform has stuff that can do that (exactly as pointed out by that issue), moving those calls doesn't necessarily solve anything here (and if anything it's a higher risk change to introduce a constructor to an abstract class where one didn't exist previously).

[shakaran]

sn't the autoloader arbitrarily including all files, but rather processes that eagerly scan and load files. I know API Platform has stuff that can do that (exactly as pointed out by that issue), moving those calls doesn't necessarily solve anything

So, since we cannot avoid it, if 2.0 get released, then we can remove all trigger_deprecation() calls finally