Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Circular dependency when using MAILER_DSN in env #10888

Closed
lekoala opened this issue Jul 27, 2023 · 7 comments
Closed

Circular dependency when using MAILER_DSN in env #10888

lekoala opened this issue Jul 27, 2023 · 7 comments

Comments

@lekoala
Copy link
Contributor

lekoala commented Jul 27, 2023

Affected Version

5

Description

When using MAILER_DSN in env, it's very easy to get a circular dependency when using "after: - framework" for example
not sure why
i've found that replacing with "#corecache" works much better, so that's what i'm currently doing, but i'm not sure about the best practice here
I guess i'm not the only one who will be facing this :-)

Steps to Reproduce

  • set a MAILER_DSN in .dev
  • have a module using after: - 'framework'
  • run dev/build => Uncaught MJS\TopSort\CircularDependencyException: Circular dependency found: my-module->mailer-dsn-env->my-module

Acceptance criteria

  • MAILER_DSN doesn't create circular injector dependencies.
  • MAILER_DSN takes precedence over everything else

Related

#8507

PRs

@GuySartorelli
Copy link
Member

For future reference: There's some context for this in silverstripe/developer-docs#92 (comment)

@lekoala
Copy link
Contributor Author

lekoala commented Jul 28, 2023

so if I understand correctly, using "after: framework" conflicts with "after: *" being used
that after: * seems really nasty for a framework level yml, i think i'm not the only one who is using expecting "after: framework" to work ?

@emteknetnz
Copy link
Member

What's causing it is this config block

From memory it was done this way so that a MAILER_DSN variable in .env would have priority over any other config, this would allow a webhost to update it rather than needing to make a code change

Agree that the after: '*' isn't a good thing to have in framework. If we are to fix seems that a good starting point would be to try to move what the yml is doing to some PHP that runs very early on in the request lifecycle and checks to see if there is a MAILER_DSN environment variable around and if so then update the Symfony\Component\Mailer\Transport\TransportInterface: injector constructor arguments

If that works OK then we can delete the contents of the yml block, though we'll probably still need to retain the yml block as an empty shell for BC in case anyone has an after: mailer-dsn-env in their project yml

@lekoala
Copy link
Contributor Author

lekoala commented Jul 31, 2023

@emteknetnz and what about making a new factory

Symfony\Component\Mailer\Transport\TransportInterface:
factory: Symfony\Component\Mailer\Transport

=> SilverStripe\Control\Email\TransportFactory

that class simply extend the current Factory and overrides, if necessary, with env var in the constructor any parameter being passed. This way, you now for a fact that env var is going to be applied

@emteknetnz
Copy link
Member

@lekoala - great idea. Have implemented #10898

@emteknetnz emteknetnz removed their assignment Jul 31, 2023
@GuySartorelli GuySartorelli self-assigned this Aug 1, 2023
@GuySartorelli
Copy link
Member

PR merged.
Because this is adding new API it will be included in the 5.1.0 release instead of being a patch on 5.0.

@lekoala
Copy link
Contributor Author

lekoala commented Aug 1, 2023

@emteknetnz awesome :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants