Skip to content

Set Server Authentication automatic when the service is called#8

Open
redjanym wants to merge 2 commits intowemakecustom:masterfrom
redjanym:master
Open

Set Server Authentication automatic when the service is called#8
redjanym wants to merge 2 commits intowemakecustom:masterfrom
redjanym:master

Conversation

@redjanym
Copy link

No description provided.

@lemoinem
Copy link
Contributor

Hello,

Thank you for your contribution.

If you take a look at

self::setApiKey($secretKey);
you will notice that self::setApiKey($secretKey); is called in the constructor.

I think it would be better if the API Key was provided through the constructor as an argument.
For now, simply calling auth won't do anything more.

arguments: [%wmc_stripe.api_publishable_key%]
tags:
- { name: twig.extension }
- { name: twig.extension } No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Please restore the empty line at the end of this file.

@lemoinem
Copy link
Contributor

lemoinem commented Mar 20, 2017

Hi,

I'm sorry for the delay since my last answer. I didn't see your second commit.

I just looked at the code itself, and I am having some issues with your modifications:

Symfony creates its services the first time they are requested.

Which means, your modification would work kind-of OK if the Stripe service is injected as a dependency in another service. However, in a Controller (using the base Controller class), the service will never get created and the API Key will never get populated, unless the service is called explicitly (and then nothing is done with it).
Even in the case of an injection in another service, you'd inject a service nothing's done with afterward, that looks weird...

This seems to me like a weird pattern.
What do you think? Did you run into any issue with the modifications you propose?

Do you think something in particular should be added in the documentation?
Having at least one method called explicitly on the service for initialization seems better and provides more explicit code.

If the service was used afterward as a Factory or a Facade for the resources, I wouldn't mind integrating the authentification step to service definition, but as it stands, it seems weird.

What do you think?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants